Step 6: Testing

Your Step 2 reproducer proved the bug exists and your Step 5 fix made it green. That single test is necessary but not sufficient. The tests you ship in the PR have a different job: they are permanent regression protection that must encode every trigger condition, survive thousands of randomized runs by other people on other machines, and convince a maintainer that the fix is correct and that the surrounding behavior is unbroken.

The rule that governs this step: a test that would have passed before your fix is not a test of your fix. Every test you add must be red on main and green with your change.

The Test Pyramid in OpenSearch

Match the test level to what the bug actually needs. Cheaper is better; reach for the in-cluster harness only when single-class testing genuinely cannot reproduce.

Unit Tests

The aggregation fix lives in one class, so its primary test is a unit test in the aggregator's existing ...TestCase. Cover the trigger conditions explicitly and add a negative control — the same shape where the bug must not fire.

/*
 * SPDX-License-Identifier: Apache-2.0
 * ... keep the existing header ...
 */
public class TermsAggregatorTests extends AggregatorTestCase {

    // THE FIX: synthetic + `missing` bucket survives when a shard has zero docs.
    public void testMissingWithMinDocCountZeroOnEmptyShard() throws Exception {
        MappedFieldType fieldType = new KeywordFieldMapper.KeywordFieldType("absent_field");
        TermsAggregationBuilder agg = new TermsAggregationBuilder("by_missing")
            .field("absent_field").missing("N/A").minDocCount(0);

        try (Directory dir = newDirectory();
             RandomIndexWriter w = new RandomIndexWriter(random(), dir)) {
            w.addDocument(List.of());                 // a doc with no value -> empty segment for field
            try (IndexReader reader = w.getReader()) {
                StringTerms result = searchAndReduce(
                    newIndexSearcher(reader), new MatchAllDocsQuery(), agg, fieldType);
                assertEquals(1, result.getBuckets().size());            // FAILS on main
                assertEquals("N/A", result.getBuckets().get(0).getKeyAsString());
                assertEquals(1, result.getBuckets().get(0).getDocCount());
            }
        }
    }

    // NEGATIVE CONTROL: with min_doc_count >= 1, the empty bucket must still be dropped.
    public void testMissingWithMinDocCountOneStillDropsEmptyBucket() throws Exception {
        MappedFieldType fieldType = new KeywordFieldMapper.KeywordFieldType("absent_field");
        TermsAggregationBuilder agg = new TermsAggregationBuilder("by_missing")
            .field("absent_field").missing("N/A").minDocCount(1);
        try (Directory dir = newDirectory();
             RandomIndexWriter w = new RandomIndexWriter(random(), dir)) {
            w.addDocument(List.of());
            try (IndexReader reader = w.getReader()) {
                StringTerms result = searchAndReduce(
                    newIndexSearcher(reader), new MatchAllDocsQuery(), agg, fieldType);
                assertEquals(0, result.getBuckets().size());            // unchanged by the fix
            }
        }
    }
}

The negative control is what separates rubric band 14–15 from 11–13: it proves your fix is scoped, not a blanket behavior change. Run them:

./gradlew :server:test \
  --tests "org.opensearch.search.aggregations.bucket.terms.TermsAggregatorTests.testMissingWithMinDocCountZeroOnEmptyShard" \
  --tests "org.opensearch.search.aggregations.bucket.terms.TermsAggregatorTests.testMissingWithMinDocCountOneStillDropsEmptyBucket"

Serialization Tests (when the wire format is involved)

If your fix touched a Writeable (it shouldn't have for the aggregation bug, but the seqno/engine response or a new cluster-state piece might), add or extend an AbstractWireSerializingTestCase. It round-trips the object through StreamOutput/StreamInput on random instances and, critically, across versions:

public class SomeResponseTests extends AbstractWireSerializingTestCase<SomeResponse> {
    @Override protected Writeable.Reader<SomeResponse> instanceReader() { return SomeResponse::new; }
    @Override protected SomeResponse createTestInstance() { /* random fields incl. the new one */ }

    // BWC: serialize at an old version, ensure the new field is absent/defaulted.
    public void testSerializationBwc() throws IOException {
        SomeResponse original = createTestInstance();
        SomeResponse roundTripped = copyInstance(original, Version.V_3_0_0);  // old version
        assertNull(roundTripped.getNewField());                              // guarded out pre-3.1
    }
}

copyInstance(instance, version) writes/reads at a pinned Version — that is the mechanical proof your Version guards from Step 5 are symmetric. See the serialization & BWC deep dive for the full treatment.

Integration Tests (when one class can't reproduce)

The ClusterApplierService listener-race and any replication/recovery bug need a real cluster. These live under server/src/internalClusterTest/java/... and run via :server:internalClusterTest, backed by InternalTestCluster.

@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 2)
public class IndexResourceLeakIT extends OpenSearchIntegTestCase {

    public void testFastCreateDeleteDoesNotLeakResource() throws Exception {
        internalCluster().startClusterManagerOnlyNode();   // cluster manager (formerly master)
        internalCluster().startDataNodes(2);

        // Drive the trigger: rapid create-then-delete so the applier may coalesce states.
        for (int i = 0; i < 50; i++) {
            createIndex("repro-" + i, Settings.builder()
                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
                .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1).build());
            client().admin().indices().prepareDelete("repro-" + i).get();
        }

        // The fix reconciles against current Metadata; assert no leaked resources remain.
        assertBusy(() -> {
            long open = someComponent().openResourceCount();   // expose via a test hook
            assertEquals("resources leaked after fast create/delete", 0L, open);
        });
    }
}

./gradlew :server:internalClusterTest \
  --tests "org.opensearch.cluster.*.IndexResourceLeakIT.testFastCreateDeleteDoesNotLeakResource"

Warning: Never wait with Thread.sleep. Cluster state settles asynchronously; a sleep is non-deterministic by construction and a reviewer will (correctly) block it. Use assertBusy(...) (retries until the assertion holds or times out), ensureGreen(...), ensureStableCluster(n), or a ClusterStateListener/ClusterStateObserver latch.

To force the coalescing/race deterministically rather than hoping the loop hits it, the test framework's disruption helpers (test/framework/.../disruption/, e.g. BlockClusterStateProcessing, SlowClusterStateProcessing) let you pause state application on a node and release it after queuing both updates.

REST-YAML Tests (the response contract)

If the bug is user-visible at the REST layer — and the aggregation bug is — a REST-YAML test is the most faithful regression guard for the contract. Add it under the relevant module's yamlRestTest resources (for core search, rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/):

---
"terms agg with missing and min_doc_count zero on empty shard":
  - do:
      indices.create:
        index: repro
        body:
          settings: { number_of_shards: 1, number_of_replicas: 0 }
  - do:
      index: { index: repro, refresh: true, body: { value: 10 } }
  - do:
      search:
        index: repro
        body:
          size: 0
          aggs:
            by_missing:
              terms: { field: absent_field, missing: "N/A", min_doc_count: 0 }
  - match: { aggregations.by_missing.buckets.0.key: "N/A" }
  - match: { aggregations.by_missing.buckets.0.doc_count: 1 }

./gradlew :rest-api-spec:yamlRestTest \
  --tests "*ClientYamlTestSuiteIT" -Dtests.rest.suite=search.aggregation

BWC Tests (only if behavior crosses versions)

Most bug fixes do not need a dedicated BWC test — the serialization round-trip test above covers wire compatibility. You add a qa/ BWC test only when the fix changes behavior that a mixed-version cluster or a rolling upgrade must tolerate (e.g. a coordination change where an old cluster manager and new data node must interoperate). These run with a bwcVersion:

./gradlew :qa:rolling-upgrade:check
./gradlew :qa:mixed-cluster:check

If your fix has no cross-version behavior change, state that explicitly in the PR — "no BWC test needed; serialization unchanged, behavior change is local to a single node's response." Reviewers would rather see that judgment than an irrelevant BWC test.

Determinism: the Non-Negotiable

OpenSearch tests run under RandomizedRunner with a random seed, random locale, and random timezone. A test that passes on your seed and fails on someone else's is worse than no test. Enforce determinism:

• No Thread.sleep. Use assertBusy, ensureGreen, latches, or ClusterStateObserver.
• No wall-clock assertions unless you inject a deterministic clock.
• No order-dependent assertions over a HashMap/Set — sort, or assert on a set, not a list order.
• Respect the seed. Build random inputs from random(), randomAlphaOfLength(...), randomIntBetween(...) — not Math.random() or new Random(). This makes failures reproducible from the printed -Dtests.seed=... line.

Prove your test is deterministic by running it many times with different seeds:

# Hammer it. iters reruns within one JVM; loop across fresh seeds.
./gradlew :server:test --tests "...testMissingWithMinDocCountZeroOnEmptyShard" \
  -Dtests.iters=50
for i in $(seq 1 10); do
  ./gradlew :server:test --tests "...testMissingWithMinDocCountZeroOnEmptyShard" \
    -Dtests.seed=$(openssl rand -hex 8) -q || echo "FLAKE on iteration $i"
done

If any iteration flakes, the test is not done. A flaky test you ship becomes a flaky-test-labeled issue and an @AwaitsFix — exactly what you do not want attached to your name. (And never mute a real flake with @Ignore; if you must mute, use @AwaitsFix(bugUrl="https://github.com/opensearch-project/OpenSearch/issues/NNNN").)

Deliverable for Step 6

• A test at the lowest level that reliably covers the bug (unit > integ > manual), red on main, green with the fix.
• A negative control proving the fix is scoped (the near-miss case still behaves as before).
• Every trigger condition from Step 2/4 encoded in a test assertion.
• A serialization round-trip + BWC test iff the wire format changed (otherwise an explicit "no serialization change" note).
• A REST-YAML test iff the bug is a REST contract.
• Determinism proven: 50 iters + 10 fresh seeds, zero flakes, no sleep.

Validation / Self-check

Before advancing to Step 7:

1. Every new test fails on main and passes with your fix — you verified by stashing the fix and re-running.
2. You included a negative control that would catch an over-broad fix.
3. You chose the lowest-cost harness that reliably reproduces; you did not write an integration test for a single-class bug.
4. No Thread.sleep, no wall-clock, no order-dependent assertions anywhere.
5. You ran the test ≥ 50 iterations and ≥ 10 seeds with no flake.
6. Serialization/BWC coverage matches your Step 5 BWC decision (test present iff wire changed; otherwise documented as unchanged).
7. Test names describe the scenario (testMissingWithMinDocCountZeroOnEmptyShard), not the method under test.

Then go to Step 7: Validation.