Skip to content

Refactor advanced_global_advanced_syntax_search_spec.rb to use API only

Erick Banks requested to merge egb-refactor-essearch-api-test into master

What does this MR do?

I've moved the qa/qa/specs/features/ee/browser_ui/enablement/elasticsearch/advanced_global_advanced_syntax_search_spec.rb to qa/qa/specs/features/ee/api/enablement/elasticsearch/advanced_global_advanced_syntax_search_spec.rb

I then refactored the test to run as an API only test. I did this because the slowest running test in this class would run in about 2 minutes. It was also the slowest running test in the elasticsearch test suite: https://gitlab.com/gitlab-org/gitlab-qa/-/jobs/451302665

126.34 seconds ./qa/specs/features/ee/browser_ui/enablement/elasticsearch/advanced_global_advanced_syntax_search_spec.rb:44

I've gotten it to run, locally, to the tune of about 0.28555 seconds. I figured a three-order of magnitude improvement was worth it (though that's hardly a fair comparison taking one value from a CI run and another from a local run, but the song remains the same, we should expect at least a 10x gain... I'll wait for the new API test to run in CI and report back how long it took and how much actual gains we benefit from).

Ok, it ran in CI and the results are (https://gitlab.com/gitlab-org/gitlab-qa/-/jobs/451505080):

Create Elasticsearch advanced global search with advanced syntax when searching for projects using advanced syntax searches in the project description
     0.53964 seconds ./qa/specs/features/ee/api/enablement/elasticsearch/advanced_global_advanced_syntax_search_spec.rb:37
   Create Elasticsearch advanced global search with advanced syntax when searching for projects using advanced syntax searches in the project name
     0.26836 seconds ./qa/specs/features/ee/api/enablement/elasticsearch/advanced_global_advanced_syntax_search_spec.rb:33

So... that runs about two hundred times faster. Nice.

But that's a little unfair, as we're just measuring the test execution and no the entire suite time including setup and teardown.

Full UI test: 89.06 seconds average (178.12 seconds / 2 examples)

API only test: 32.57 seconds average (65.13 seconds / 2 examples)

Which is still a good time increase of about 60% faster. Also, I suspect the test could run faster if we implement this issue (https://gitlab.com/gitlab-org/quality/team-tasks/issues/395) that can monitor the logs and start the test once we know that elasticsearch will be tagging new records to be searchable. Then we could modify the test to wait smarter and start execution of tests sooner. Also, also, I suspect the tests are hitting their 60 second sleep in the before blocks as the pipelines do not seem to be started with elasticsearch already turned on, perhaps we can change the pipeline so that elasticsearch already on (that is a GET settings API call should return "elasticsearch_search":true and "elasticsearch_indexing":true) before starting the tests.

Once the speed was acceptable, I then moved some of the methods from the test files to the QA::Support::Api class because we're using that for get and put calls as well as http statuses already. I'm not 100% convinced this is the right place for those methods, but we can move them if we find a better home (one candidate being qa/qa/ee/resource/settings/elasticsearch.rb). This let me reduce the number of lines of code in the original elasticsearch_api_spec.rb test from 104 to 71, or about 30% fewer lines of code.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

The risk here is that we're assuming the API and the UI exercise the same underlying search logic as the browser does.

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by 🤖 GitLab Bot 🤖

Merge request reports