Skip to content

fix: key Tia results per dataset row - #1799

Open
jeffreyvanhees wants to merge 1 commit into
pestphp:5.xfrom
jeffreyvanhees:fix/tia-result-key-includes-dataset
Open

fix: key Tia results per dataset row#1799
jeffreyvanhees wants to merge 1 commit into
pestphp:5.xfrom
jeffreyvanhees:fix/tia-result-key-includes-dataset

Conversation

@jeffreyvanhees

@jeffreyvanhees jeffreyvanhees commented Jul 30, 2026

Copy link
Copy Markdown

Hi @nunomaduro!

I watched Laracon US and I've been putting TIA to work immediately on a fairly large Laravel suite, but I noticed the replay didn't really agree with the run that recorded it:

# recording
Tests: 4 skipped, 2371 passed

# replay, nothing changed in between
Tests: 6 skipped, 2369 passed

Two tests that had passed came back as skipped. A plain --no-tia run agrees with the recording, so the replay was the odd one out.

The cause is that the result cache is keyed on Class::method and never sees the dataset. It happens in three spots:

  • EnsureTiaResultsAreCollected writes $test->className().'::'.$test->methodName()
  • EnsureTiaAssertionsAreRecordedOnFinished builds the same string
  • Testable reads it back as $this::class.'::'.$this->name(), and TestCase::name() gives you the bare method name

So every row of a ->with() test writes into the same entry and the last one to finish wins. On replay that one status gets handed to all the rows without running any of them. In my case three tests call markTestSkipped() for one harness only, so the skip got reported for the sibling row too, which is where the extra two came from.

The counts are the harmless half of it. Graph::shouldRerunStatus() re-runs a cached failure, so red in the cache is fine, but a cached pass gets replayed onto its siblings, and a row that would have failed never runs. I haven't hit that in practice, since a change that breaks a row usually marks the file affected anyway, but it seemed worth mentioning.

The fix is small: TestMethod::id() already appends #dataSetName when the test comes from a data provider, and TestCase::valueObjectForEvents() hands you the same value object, so both sides can just use that. Tests without a dataset keep the key they have today.

Afterwards the replay lines up:

# recording
Tests: 4 skipped, 2371 passed

# replay
Tests: 4 skipped, 2371 passed

A few notes:

  • Baselines recorded before this use the old keys, so they won't match and get re-recorded. The graph already handles that.
  • The new test covers all three call sites. The first two fail on 5.x (the first one prints the two rows collapsing into a single key), the third pins that a test without a dataset is untouched.
  • composer test:unit has one failure on Tests\Unit\Support\Backtrace, but it fails on a clean 5.x here too, so it isn't from this change.

Happy to adjust anything, and thanks for the TIA-feature, the speedup is awesome!

The result cache was keyed `Class::method` on both sides, so every row of a
`->with()` test shared one entry and the last writer won. On replay that single
status was handed to every row.

`TestMethod::id()` already appends `#dataSetName`, and `TestCase` reaches the
same value object through `valueObjectForEvents()`, so both sides can use it.
@jeffreyvanhees
jeffreyvanhees marked this pull request as ready for review July 30, 2026 19:02
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.

1 participant