Integration test runner #12
Conversation
JoshMock
force-pushed
the
integration-tests
branch
from
3906fc2 to
7e80313
Compare
With fixes for several test breakages. Integration tests will always run against a real serverless instance that has its own certs. That instance is created using qaf, one per test suite run (so several are created for each matrix-based test run), and then deleted when tests are done running.
Since serverless projects provide users with an endpoint that does not include a port, we should assume HTTPS means port 443.
Instead of using the entire set of YAML REST tests from the elasticsearch repo.
JoshMock
force-pushed
the
integration-tests
branch
from
503f9fb to
8e642ab
Compare
Was using a specific commit, which has been merged to main.
| EC_REGISTER_BACKEND: "appex-qa-team-cluster" | ||
| EC_ENV: "qa" | ||
| EC_REGION: "aws-eu-west-1" | ||
| EC_PROJECT_NAME: "esv-client-python-test-{{ matrix.python }}-{{ matrix.suite }}-{{ matrix.connection_class }}-{{ key }}" |
There was a problem hiding this comment.
Another improvement I hope to make soon is to spin up a single serverless project for the whole matrix, ensuring each test run in the matrix uses a unique prefix (probably the Buildkite step ID) on all resources created in Elasticsearch. It's slow, ugly and expensive to set up several serverless instances every time this test suite is run.
| async_client, | ||
| "test_index", | ||
| "prod_index", | ||
| bulk_kwargs={"refresh": "wait_for"}, |
There was a problem hiding this comment.
Refreshing is costly in serverless as it requires going to the object store. Thankfully grepping for refresh in the test only finds test_helpers and one entry in test_elasticsearch_serverless/test_async/test_server/test_mapbox_vector_tile.py. But Is there any particular reason to use ?refresh=wait_for here? I know I've advocated for this before, but I thought it was much more efficient than it is.
Indeed, refresh=wait_for waits for the next refresh interval, which is currently 15s! refresh=True has burst capacity, and in the worse case it will be throttled to one refresh per 5 seconds, which sounds better here.
There was a problem hiding this comment.
I think I was just trying to figure out how to update this refresh to ensure the test I copied from elasticsearch-py would keep passing. I intend to address all refreshes in a follow-up because I believe that change is the cause of quite a few test failures.
| @@ -114,22 +63,12 @@ def wipe_cluster(client): | |||
|
|
|||
| if isinstance(client, AsyncElasticsearch): | |||
| node_config = client.transport.node_pool.get().config | |||
| client = Elasticsearch(node_config, verify_certs=False) | |||
| client = Elasticsearch([node_config]) | |||
| close_after_wipe = True | |||
| except ImportError: | |||
| pass | |||
|
|
|||
| is_xpack = True | |||
There was a problem hiding this comment.
Since is_xpack is unconditionally True, why do we still have conditions below?
There was a problem hiding this comment.
Good point, will clean up soon.
Gets integration test suite running, including fixing a few bugs due to copy/pasting from the stack client. And adds a Buildkite pipeline for it.
Several pieces of stack-specific functionality have been removed from tests, and dependence on an API key has been added to properly reflect Serverless.
Not all of these test pass yet. Many async tests are flaky due to setup/teardown issues in the ES instance, mapbox vector tile tests fail because I need to regenerate the API from latest spec changes, and there are a few places where I haven't landed on the best way to refresh indices. I'll fix these issues in separate PRs.
A couple secrets still need to be created in the CI Vault instance:
secret/ci/elastic-elasticsearch-serverless-python/cloud-accesssecret/ci/elastic-elasticsearch-serverless-python/github-tokenI can't create them until this merges because https://github.com/elastic/catalog-info/pull/558 needs the catalog-info.yaml file created in this PR.