Step 6: Testing

Your Step 2 reproducer proved the bug exists and your Step 5 fix turned 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, run deterministically on every machine, and convince a committer 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 master and green with your change — and you prove it by stashing the fix and re-running.

You write the regression test first, against the unfixed code (you already did, in Step 2). Step 6 hardens that test to shippable quality, adds the branch coverage and negative controls a reviewer expects, and proves it is not flaky.

Test-type selection

Match the test level to what the bug actually needs. Cheaper is better; reach for MiniTezCluster only when a controlled-dispatcher unit test genuinely cannot reproduce. Every base class and example path below is real in the checkout — open them.

A state-machine bug reproduced as a MiniTezCluster job costs you 90 seconds an iteration forever. A shuffle-correctness bug tested only as a unit test proves nothing about the wire. Pick the lowest level that reliably reproduces — and, for a correctness or recovery bug, add one integration test on top so the fix is proven end-to-end.

Unit tests with a controlled dispatcher

The single most important Tez test pattern: synchronous, deterministic state-machine testing. DrainDispatcher is Hadoop's synchronous dispatcher — you dispatch() events into a queue, and await() drains the queue on the calling thread so every handler completes before await() returns. Two superpowers follow: deterministic event ordering (dispatch A, dispatch B, await → A's handler finished before B's started) and no real threading (the bug reproduces on every machine, not only under contention). Read the canonical example before you write:

grep -nE "DrainDispatcher|dispatcher.await\(\)|createVertex\(" \
  tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestVertexImpl.java | head -30

The state-transition test template

Every state-machine unit test follows the same visible shape — arrange, set precondition, act, assert. Reviewers skim for that shape; do not hide setup inside helpers that make a future failure hard to debug.

@Test(timeout = 10_000)
public void testV_TASK_COMPLETED_inRunningWithRecovery_doesNotShortCircuit()
    throws Exception {
  // 1. Arrange: drive the SUT to the state under test.
  vertex.handle(new VertexEvent(vertex.getVertexId(), VertexEventType.V_INIT));
  dispatcher.await();
  vertex.handle(new VertexEvent(vertex.getVertexId(), VertexEventType.V_START));
  dispatcher.await();
  assertEquals(VertexState.RUNNING, vertex.getState());

  // 2. Set the precondition that triggers the bug (a Step-4 trigger condition).
  vertex.setRecoveryData(mockRecoveryData());

  // 3. Act: fire the event under test.
  TezTaskID lastTaskId = vertex.getTask(vertex.getNumTasks() - 1).getTaskId();
  vertex.handle(new VertexEventTaskCompleted(lastTaskId, TaskState.SUCCEEDED));
  dispatcher.await();

  // 4. Assert: the new state AND the side-effect counters.
  assertEquals(VertexState.SUCCEEDED, vertex.getState());
  assertEquals(vertex.getNumTasks(), vertex.getCompletedTaskCount());
}

Reuse TestVertexImpl's existing createVertex(...) / setUp() collaborators — do not rebuild the AppContext / event-handler / EdgeManager zoo. If a test class already exists for your class (it does, for every impl), add a @Test method there rather than a new file.

Test both branches of every guard you add

If your Step 5 fix introduced a guard:

if (recoveryData != null && isReplayingRecovery()) { ... }

you owe a test for each branch it can take. Enumerate them in a table so you can see the gaps:

The third row is your fix. The others are the negative controls that prove the fix is scoped — that you did not change behavior on the paths that were already correct. The negative control is what separates a rubric-band-14 test suite from a band-11 one: it proves a future refactor that deletes your guard will turn a test red.

Scheduler unit tests: the drainable-callback pattern

Scheduler bugs do not need a cluster. TEZ-4580 (slow preemption when reuse is enabled) added exactly one @Test(timeout = 5000) to TestTaskScheduler that drives a mocked RM and asserts on verify(mockRMClient, times(N)).... Read it end to end — it is the gold-standard template for this whole class of bug:

git show 9efa6f14d -- tez-dag/src/test/java/org/apache/tez/dag/app/rm/TestTaskScheduler.java

The load-bearing move is drainableAppCallback.drain() after each action, which flushes the async callback queue so the verify(...) interaction counts are deterministic — the scheduler-side analogue of dispatcher.await(). The assertions are on RM interactions (addContainerRequest, releaseAssignedContainer), not on wall time, which is what makes a preemption-timing test deterministic. See the scheduler deep dive.

Integration tests on MiniTezCluster

Unit tests prove the fix works in isolation. For correctness, shuffle, and recovery bugs, add one integration test that proves it works wired to a real (in-JVM) YARN cluster. The setup/teardown contract:

private static MiniTezCluster tezCluster;

@BeforeClass
public static void setup() throws Exception {
  Configuration conf = new Configuration();
  tezCluster = new MiniTezCluster(TestTezNNNNFix.class.getName(),
      /*numNodeManagers=*/ 1, /*numLocalDirs=*/ 1, /*numLogDirs=*/ 1);
  tezCluster.init(conf);
  tezCluster.start();
}

@AfterClass
public static void tearDown() {
  if (tezCluster != null) { tezCluster.stop(); tezCluster = null; }
}

@Test(timeout = 180_000)
public void testTezNNNNFixEndToEnd() throws Exception {
  TezConfiguration tezConf = new TezConfiguration(tezCluster.getConfig());
  DAG dag = buildDAGThatExercisesFix();

  TezClient client = TezClient.create("test-tez-NNNN", tezConf);
  client.start();
  try {
    DAGClient dagClient = client.submitDAG(dag);
    DAGStatus status = dagClient.waitForCompletionWithStatusUpdates(
        EnumSet.of(StatusGetOpts.GET_COUNTERS));
    assertEquals(DAGStatus.State.SUCCEEDED, status.getState());

    // The assertion that proves the fix works end-to-end — a counter, not a log line.
    long counter = status.getDAGCounters()
        .findCounter(DAGCounter.NUM_SUCCEEDED_TASKS).getValue();
    assertEquals(expectedTasks, counter);
  } finally {
    client.stop();
  }
}

For a recovery bug, model your test on TestAMRecoveryAggregationBroadcast (TEZ-4569) — it wires MiniDFSCluster + MiniTezCluster, submits a real MR-input DAG, kills the AM, and asserts the DAG still succeeds:

git show 44c4f1ec9 -- tez-tests/src/test/java/org/apache/tez/test/TestAMRecoveryAggregationBroadcast.java

waitForCompletionWithStatusUpdates(Set<StatusGetOpts>) is the real DAGClient API — it blocks until the DAG finishes. Never poll with Thread.sleep.

Test quality gates

Every test you ship must pass all four gates. A test that fails any of them gets the PR sent back.

The determinism gate is the one Tez enforces hardest — the project has shipped many flaky-test fixes, and almost all of them replace a Thread.sleep with an event-driven await or replace a counter assertion with a state assertion. Read a couple to see the shape:

git log --oneline --grep="flaky" --grep="intermittent" -i | head -20
git show <one-of-those-shas>

See the flaky-test stage for the full treatment.

Test naming

Tez convention — the verbose name is the test's documentation, and future-you reading CI output will be glad for it:

• File: Test<ClassUnderTest>.java under <module>/src/test/java/<package>/. If TestVertexImpl.java already exists, add a @Test method there — do not create a parallel file.
• Method: test<Event>_<State>_<ExpectedResult> or test<Scenario>_<Behavior>.
• Bad: testFoo, testBug, testCase1.
• Good: testV_TASK_COMPLETED_inRunningWithRecoveryData_doesNotShortCircuit.

Run the surrounding module's full suite

Your new test passing is not enough — you must prove you did not break a neighbor. Before you push, run the whole module the fix lives in, not just your test:

# The module you changed. This is the one that must be fully green.
mvn test -pl tez-dag

# If your fix touches API or common, downstream modules depend on it — run them too.
mvn test -pl tez-tests -Dtest='TestLocalMode,TestOrderedWordCount'

Some pre-existing flakes are normal in tez-dag; a run that fails only on tests you never touched, reproducibly failing on master too, is not your regression. Prove that by running the same suite on a clean master and diffing the failure sets. A test you touched that fails is always your problem.

Checking for flakiness

A flaky test you ship is technical debt every other contributor pays, and it will be reverted with your name on it. Prove your tests are not flaky before pushing — run them in a tight loop:

for i in $(seq 1 20); do
  echo "=== Run $i ==="
  mvn test -pl tez-dag -Dtest=TestVertexImplTezNNNN -q || { echo "FLAKE on run $i"; break; }
done

Twenty consecutive green runs for a unit test; ten for an integration test (they are slower). If even one run fails, you have a flaky test — fix the non-determinism (it is almost always a hidden sleep, an unpinned Random, or an order-dependent assertion on an unordered collection), do not paper over it. Never mute a real flake to get a merge; muting a flake you introduced is the worst possible outcome for your name.

Optional: coverage on the lines you changed

You do not need 100% line coverage on the file — you need every new branch exercised on both sides. Tez has a jacoco Maven profile you can turn on to spot-check:

mvn test -pl tez-dag -Dtest='TestVertexImpl*' -Pjacoco
# then open tez-dag/target/site/jacoco/index.html and look at the lines you changed

If a line you added shows red, add a test that covers it before you push.

What reviewers look for in Tez test additions

Derive the bar from what actually merged. Read the test halves of the two exemplar commits (git show 9efa6f14d, git show 44c4f1ec9) and you will see the same reviewer expectations every time:

• The test is red without the production change. Reviewers stash your *.java production diff and re-run; if the test still passes, it is not testing your fix.
• @Test(timeout = ...) on anything that could hang — 5000ms for a scheduler unit test, 180000ms for a MiniTezCluster integration test. An untimed test that wedges hangs the whole CI run.
• Deterministic synchronization, never Thread.sleep — drainableAppCallback.drain() in the scheduler test, event await in the state-machine test, waitForCompletion... in the cluster test.
• Assertions on state/counters/interactions, not on log strings — assertEquals on getState(), verify(mockRMClient, times(N)), a DAGCounter value.
• The test lives in the right home — a new @Test on the existing Test<Class>, the ASF license header intact (TEZ-4711 normalized these; spotless will fail the build if yours is wrong).
• A negative control proving the fix is scoped, wherever a guard was added.

Deliverable for Step 6

• A regression test at the lowest level that reliably covers the bug, red on master, green with the fix — verified by stashing the fix.
• A negative control for every guard you added, proving the fix is scoped.
• Every trigger condition from Steps 2 and 4 encoded in an assertion.
• An integration test on local mode or MiniTezCluster iff behavior is end-to-end (correctness, shuffle, recovery).
• @Test(timeout=...) on every test; no Thread.sleep, no unpinned Random, no order-dependent assertions.
• The full mvn test -pl <module> suite green (minus documented pre-existing flakes).
• 20 consecutive green runs (unit) / 10 (integration), zero flakes.

Validation / Self-check

Before advancing to Step 7:

1. Every new test fails on master and passes with your fix — you verified by stashing the fix and re-running.
2. At least one test uses DrainDispatcher / a drainable callback for deterministic ordering (or has a documented reason it does not — a pure, event-free unit).
3. At least one integration test (local mode or MiniTezCluster) is present if the fix affects end-to-end behavior.
4. Every new conditional branch in your production code has a test exercising each side, including a negative control.
5. No Thread.sleep, no wall-clock waits, no unseeded Random, no order-dependent assertions on unordered collections.
6. 20 consecutive runs (unit) / 10 (integration) are all green.
7. mvn test -pl <module> runs the whole module green, and you have confirmed any remaining failures also fail on a clean master.
8. Test names describe the scenario, not just the method under test.

Then go to Step 7: Validation.