Skip to content

Conversation

@bhapas
Copy link
Contributor

@bhapas bhapas commented Jun 29, 2023

This PR changes the way the system testing asserts at the number of hits in ElasticSearch when a system test is run.

Earlier, The testrunner exited when enough docs are found as per the config and there were more docs that got ingested after assertHitCount condition satisfied.

Now, The testrunner tries to get all the docs before checking for the assertHitCount and fails if the docs are more/less than the configured count

The retryTicker in the waitUntilTrue function is changed to 5 seconds meaning , the retry interval for each poll to ES has been changed to 5 seconds and so if for 10 seconds, there is no change in number of docs , the test passes.

How to test

Run PACKAGE_TEST_TYPE=false_positives PACKAGE_UNDER_TEST=httpjson_false_positive_asserts ./scripts/test-check-packages-false-positives.sh

The test-check-packages.sh file has been modified to catch a failure from the new package httpjson_false_positive_asserts and a testcase failure is considered as success scenario

@bhapas bhapas requested a review from mrodm June 29, 2023 10:30
@bhapas bhapas self-assigned this Jun 29, 2023
@bhapas bhapas requested a review from marc-gr June 29, 2023 10:31
@bhapas bhapas requested a review from andrewkroh June 29, 2023 19:34
@bhapas bhapas requested a review from jsoriano June 30, 2023 09:03
@bhapas bhapas changed the title [system testing] Wait until all the hits in the index are found [system testing] Fix false positives in assert.hit_count Jun 30, 2023
@bhapas bhapas force-pushed the false-negatives branch from 55424be to 0a0094f Compare June 30, 2023 09:37
@jsoriano
Copy link
Member

test integrations

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to have the first negative test 🙂

I think these changes are going to affect the timings of all tests, and not only the ones with hit count assertions, could we change it so these changes only affect tests with hit count assertions?

@mrodm
Copy link
Contributor

mrodm commented Jul 3, 2023

test integrations

@elasticmachine
Copy link
Collaborator

Created or updated PR in integrations repostiory to test this vesrion. Check elastic/integrations#6778

@bhapas bhapas requested a review from jsoriano July 3, 2023 09:57
@bhapas
Copy link
Contributor Author

bhapas commented Jul 3, 2023

test integrations

@elasticmachine
Copy link
Collaborator

Created or updated PR in integrations repostiory to test this vesrion. Check elastic/integrations#6778

@bhapas bhapas requested review from jsoriano and mrodm July 3, 2023 11:42
@jsoriano jsoriano dismissed their stale review July 3, 2023 11:49

Changes I requested have been addressed.

@bhapas bhapas force-pushed the false-negatives branch 2 times, most recently from cb05693 to c3808e2 Compare July 4, 2023 05:53
@bhapas
Copy link
Contributor Author

bhapas commented Jul 4, 2023

test integrations

@elasticmachine
Copy link
Collaborator

Created or updated PR in integrations repostiory to test this vesrion. Check elastic/integrations#6778

@bhapas bhapas force-pushed the false-negatives branch from c3808e2 to 4d2190c Compare July 4, 2023 07:13
@bhapas bhapas requested a review from mrodm July 4, 2023 07:52
@bhapas bhapas force-pushed the false-negatives branch from 877b5ce to 68fb07a Compare July 4, 2023 09:21
@bhapas
Copy link
Contributor Author

bhapas commented Jul 4, 2023

/test

@bhapas bhapas requested a review from mrodm July 4, 2023 14:09
Co-authored-by: Mario Rodriguez Molins <marrodmo@gmail.com>
@bhapas bhapas requested a review from mrodm July 5, 2023 06:12
@bhapas bhapas requested a review from mrodm July 5, 2023 12:43
@bhapas bhapas changed the title [system testing] Fix false positives in assert.hit_count [system testing] Add provision for system testing negative / false-positive scenarios in packages Jul 5, 2023
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @bhapas

Copy link
Contributor

@mrodm mrodm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks for adding this negative tests @bhapas!

@bhapas bhapas merged commit 604c606 into elastic:main Jul 5, 2023
@bhapas bhapas deleted the false-negatives branch July 5, 2023 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.10-candidate enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce false negatives with assert.hit_count

4 participants