Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create ES index in search_test before relying on it #3303

Merged
merged 1 commit into from
Dec 23, 2022

Conversation

martinemde
Copy link
Member

@martinemde martinemde commented Dec 20, 2022

There was an order dependent failure caused by test/integration/search_test.rb using the index without creating it.

We now create the index in that test. Any future tests that need the index should make sure to create it.

@codecov
Copy link

codecov bot commented Dec 20, 2022

Codecov Report

Merging #3303 (2066aa0) into master (7846ee1) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3303   +/-   ##
=======================================
  Coverage   98.52%   98.52%           
=======================================
  Files         113      113           
  Lines        3390     3390           
=======================================
  Hits         3340     3340           
  Misses         50       50           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

test/test_helper.rb Outdated Show resolved Hide resolved
@martinemde martinemde force-pushed the martinemde/cleanup-es-index-creation-in-tests branch from 2733ba8 to e9cd31a Compare December 21, 2022 16:58
@martinemde martinemde changed the title Delete ES index after test run to avoid leaking state between runs Create ES index once before test run Dec 22, 2022
@martinemde
Copy link
Member Author

martinemde commented Dec 22, 2022

As discussed in #3307, this blocks running individual files/tests without ES running. Is that a worthwhile trade-off or should I just fix the places where the index needs to be created first? We will just need to keep on top of which tests need the index and add that line to the setup.

@simi
Copy link
Member

simi commented Dec 22, 2022

Can you please revert change in test helper and just create index where needed for now?

Although usually this worked, it could lead to order dependent failures
when the index hadn't been created by another test at some point.
@martinemde martinemde force-pushed the martinemde/cleanup-es-index-creation-in-tests branch from e9cd31a to 2066aa0 Compare December 22, 2022 23:37
@martinemde martinemde changed the title Create ES index once before test run Create ES index in search_test before relying on it Dec 22, 2022
@martinemde
Copy link
Member Author

Done. Thanks for the specific feedback.

@simi simi merged commit c3e5226 into master Dec 23, 2022
@simi simi deleted the martinemde/cleanup-es-index-creation-in-tests branch December 23, 2022 00:07
@rubygems-org-shipit rubygems-org-shipit bot temporarily deployed to staging December 23, 2022 00:14 Inactive
@rubygems-org-shipit rubygems-org-shipit bot temporarily deployed to production December 30, 2022 22:07 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants