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

Cleanup integration tests #5429

Merged
merged 9 commits into from
Sep 20, 2024
Merged

Cleanup integration tests #5429

merged 9 commits into from
Sep 20, 2024

Conversation

rdettai
Copy link
Contributor

@rdettai rdettai commented Sep 16, 2024

Description

Uncomment some tests and remove some outdated workarounds in the integration tests:

  • ingest retries do not seem to be required anymore
  • isolated the failing test when ingesting data on a re-created index (and annotate it with #[ignore])
  • grouped all the ingest tests into a single module (ingest_tests)
  • better wording and small helper improvements

How was this PR tested?

Describe how you tested this PR.

Copy link

github-actions bot commented Sep 16, 2024

On SSD:

Average search latency is 0.998x that of the reference (lower is better).
Ref run id: 3546, ref commit: b046433
Link

On GCS:

Average search latency is 0.509x that of the reference (lower is better).
Ref run id: 3547, ref commit: b046433
Link

@@ -187,30 +162,5 @@ async fn test_multi_nodes_cluster() {
.unwrap();
assert_eq!(search_response_empty.num_hits, 0);

// Check that ingest request send to searcher is forwarded to indexer and thus indexed.
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this getting removed? That seems like a reasonable test case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ingest is tested from many angles in the dedicated ingest_tests.rs. I want basic_tests.rs to focus on basic cluster functions (start, healthchecks, empty cluster state).

@rdettai rdettai force-pushed the cleanup-integ-tests branch from 23b9fe7 to ee66453 Compare September 20, 2024 08:47
@rdettai rdettai enabled auto-merge (squash) September 20, 2024 08:47
@rdettai rdettai merged commit 634a1bc into main Sep 20, 2024
5 checks passed
@rdettai rdettai deleted the cleanup-integ-tests branch September 20, 2024 09:00
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.

2 participants