Skip to content

test: Serialize search tests, cap index waits, skip internal CI package - #4740

Open
EspenAlbert wants to merge 7 commits into
masterfrom
task/t04-13-m50-sequential-search-index-validation
Open

EspenAlbert wants to merge 7 commits into
masterfrom
task/t04-13-m50-sequential-search-index-validation

Conversation

@EspenAlbert

@EspenAlbert EspenAlbert commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Why

Both search packages fail often against cloud-dev. The two packages share one three-node M10 execution cluster and every test runs in parallel, so index builds queue. searchindexapi makes it worse: its generated resource waits three hours for READY, STEADY per operation, unconditionally, which holds a job and can cross the 300 minute test deadline. That wait is what makes this a cost change and not only a flake fix.

The same wait is opt-in in the legacy searchindex resource behind wait_for_index_build_completion, and no test in that package ever set it, so neither its create nor its update wait had run.

What

Follows the search-test 1-pager, which carries the measurements and the rejected alternatives: One Pager: Search acceptance test fixes and WRITING-40266.

This PR carries all four test-side changes from the 1-pager.

  • Skip searchindexapi in CI. acc.SkipTestForCI(t) is the first statement in all 9 tests of the package, above the acc.ClusterNameExecution call. The package stays in the autogen_slow ACCTEST_PACKAGES list, so it still runs locally and on demand and still detects upstream schema changes. Its 3 hour wait holds the job for hours, and the resource is internal: not registered in the provider, no customer docs page.
  • Serial execution. 16 of the 18 tests in searchindex and all 9 in searchindexapi move from resource.ParallelTest to resource.Test. The other 2 legacy tests already ran serially. Each package runs in its own job, so each is serialized independently.
  • Cover the build waits. TestAccSearchIndex_updatedToEmptyMappingsFields now sets wait_for_index_build_completion in both steps, so it is the first test to exercise the create and update READY, STEADY waits in the legacy package. No new test.
  • Bound the failure path. Every test config in both packages gets a timeouts block with create, update, and delete at 60 minutes: 9 configs in each package. The tests inherit no timeout today and the resource default is 3 hours. The block caps one Terraform operation, so it does not bound the multi-step test total and does not change shipped defaults.
  • Loader budget and reason. The sample-data load polls for 30 minutes instead of 15, and a FAILED load now reports the job's errorMessage, so the CI log names the cause instead of only the job ID and state. The fail-fast on FAILED is unchanged.

Type of change:

  • Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
  • New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
  • Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR.
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the contributing guides
  • I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
  • I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
  • I have added any necessary documentation (if appropriate)
  • I have run make fix and verified my code
  • If changes include deprecations or removals I have added appropriate changelog entries.
  • If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

FollowUp/Next Steps

  • The READY, STEADY misread stays open. The 1-pager reads it as the provider missing a terminal state rather than the build never finishing, and skipping searchindexapi removes the only CI reproduction. Reinstating those tests is the way back to a signal.
  • No package-level M10 figure for searchindexapi on dev, so there is no full before-and-after for the serialization there. Five of nine tests have no clean M10 measurement.
  • Helper changes are test-only. No shipped resource default changes.

@EspenAlbert EspenAlbert changed the title DO NOT MERGE: experiment with instance size for search tests test: Serialize search tests, cap index waits, skip internal CI package Sep 18, 2026
@EspenAlbert
EspenAlbert requested a lite review from Copilot September 18, 2026 11:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

internal/testutil/acc/atlas.go does not compile, and its cluster-tier override expands the stated PR scope.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates search acceptance tests to reduce contention, bound long-running operations, skip internal API tests in CI, and improve sample-load diagnostics.

Changes:

  • Serializes search acceptance tests and adds 60-minute operation timeouts.
  • Skips searchindexapi tests in CI.
  • Extends sample-data polling and reports failure details.
  • Adds configurable test cluster tiers.
File summaries
File Summary
internal/testutil/acc/cluster.go Adds configurable cluster-tier selection.
internal/testutil/acc/cluster_test.go Tests cluster-tier resolution.
internal/testutil/acc/cloud_backup_collection_restore_fixture_test.go Updates sample-load failure tests.
internal/testutil/acc/atlas.go Applies the configured cluster tier.
internal/testutil/acc/advanced_cluster.go Improves sample-data polling and error reporting.
internal/serviceapi/searchindexapi/resource_test.go Serializes, skips CI execution, and adds timeouts.
internal/service/searchindex/resource_search_index_test.go Serializes legacy tests, adds timeouts, and covers wait behavior.
Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/testutil/acc/atlas.go Outdated
Comment thread internal/testutil/acc/atlas.go Outdated
@EspenAlbert
EspenAlbert marked this pull request as ready for review September 18, 2026 12:33
@EspenAlbert
EspenAlbert requested a review from a team as a code owner September 18, 2026 12:33
@augmentcode

augmentcode Bot commented Sep 18, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR reduces search-index acceptance-test flakiness and excessive CI runtime caused by queued index builds on a shared execution cluster.

Changes:

  • Runs legacy searchindex acceptance tests serially instead of using resource.ParallelTest.
  • Skips all internal-only searchindexapi acceptance tests when running in CI, while retaining local/on-demand coverage.
  • Adds one-hour create, update, and delete Terraform timeouts to all search-index test configurations.
  • Enables wait_for_index_build_completion in the mappings-fields update test to cover legacy create and update wait paths.
  • Extends sample-data loading retries from 15 to 30 minutes.
  • Includes Atlas's sample-data job error message when a load reaches FAILED, with unit-test coverage for that message.

Technical Notes: The timeout blocks only affect acceptance-test configurations; provider defaults are unchanged. The API resource's existing multi-hour READY/STEADY waits are bounded in local test configurations and avoided in routine CI.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

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