Test Impact Analysis - #6919
Test Impact Analysis#6919sebastianbergmann wants to merge 65 commits into
Conversation
sebastianbergmann
added
type/enhancement
c866112 to
a703d1f
Compare
API Surface ChangesIf any of the additions below are not intended as public API, mark them with New API SurfaceClasses
Methods
Modified API SurfaceMethods
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6919 +/- ##
============================================
+ Coverage 99.49% 99.51% +0.01%
- Complexity 9785 10341 +556
============================================
Files 952 973 +21
Lines 29753 31111 +1358
============================================
+ Hits 29604 30960 +1356
- Misses 149 151 +2 ☔ View full report in Codecov by Harness. |
ec50b37 to
bdf152b
Compare
248a176 to
5aad389
Compare
What is recorded for a test todayThree things:
The gapCode that a test executes, that is not first-party code under public function testAdds(): void
{
$this->assertSame(Helper::expectedSum(), (new Calculator)->add(1, 2));
}What gets recorded for that test is That is the bad shape: the suite reports success while a change that breaks it goes unrun. It is not a stale-data problem; the recording is up to date and correct about everything it covers. What this is not
WorkaroundAdding the helper directory to The cost is that the helpers then count as first-party code for code coverage too, and show up in coverage reports. Directions considered, none chosen (yet)
The first two change what "what a test depends on" means, so they want a deliberate decision rather than being settled on the way past. |
40fc113 to
5a7b2ae
Compare
|
When Being affected by a change is only one of the reasons a test is run. A test is also run when nothing is known about it, and that number can dominate the selection without anything saying so. Read as it stands, the line above claims something about 44 tests that is only true of a few of them. The new A test is reported under one of these:
That second reason is usually the surprising one. Tests that are always skipped never record anything, so they stay in the selection no matter what you change. They cost almost nothing to run, but until now there was no way to see that this is what was happening. The option reports exactly what a run with |
…nge affects can be worked out later
…ts tests declare, instead of from what tests execute
…ss in the test index
…out the configuration, the first-party code and the installed packages
…anged, running every test whenever that cannot be determined
… of working that out from what was recorded
…file or from standard input
…amed decide which tests are selected
…n the test run ran every test there is
… for recording test impact data
…ts did not assess
…g can be affected by what changed
…ge driver is available
…erage metadata is required for all tests
… and why each of them can be
fef6829 to
09b54d8
Compare
|
This should be merged after #6784. It merges cleanly into 1. The test selection must reach the parallel test runnerThis branch passes the tests selected by The parallel branch replaces the body of If the conflict is resolved by simply keeping both sides, 2. The test impact data recorded inside a worker must reach the parentTest impact data is recorded per test in The parallel branch's workers do not do this. To fix this:
|
1 participant
The changes proposed here implement test impact analysis: PHPUnit records which source files each test depends on, and can then run only the tests a change can affect.
Closes #6897.
What it does
Recording
--record-test-impact-data, orrecordTestImpactData="true", records while the tests run which source files each test depends on. The data goes into its own file in the cache directory, separate from the test run history.Querying
--list-tests-that-depend-on src/Money.phpqueries what was recorded, without running a single test:The two lists are kept apart on purpose: the first describes the code as it is now, the second describes tests recorded against a version of that file which no longer exists.
Selecting
--only-impactedruns only the tests a change can affect:The line says how many were left out, and the summary reports the tests that actually ran. Nothing pretends the other 151 passed.
PHPUnit works out what changed by comparing what is there now against what it recorded.
--impacted-by src/Money.php --impacted-by src/Servicetells it instead and it implies--only-impacted. What you name is the change set: the recorded hashes are not consulted at all. That matters on a fresh checkout, where comparing against the recording reports everything that changed since, whilegit diff --name-onlyreports what you actually did.Because naming paths one at a time does not compose with the tool that knows what changed,
--impacted-by-filereads a list, one path per line,-being standard input:An empty list is an answer rather than a missing one: nothing changed, so nothing that depends on code runs. A list that cannot be read is not an empty list, and PHPUnit stops rather than guessing.
Two ways of maintaining the data
Observation needs code coverage data collection and works no matter what a project declares.
--derive-test-impact-data-from-coverage-targetsis the alternative for projects that already take coverage targets seriously: it works out what each test depends on from the#[CoversClass]and#[UsesClass]attributes it declares, without code coverage data collection, and without the tests having been run even once.beStrictAboutCoverageMetadatais what makes it sound: under it, a test that executes code it does not declare is already marked risky, so declarations are a superset of execution. PHPUnit warns when the mode is used without the strict check, because then nothing has ever verified the declarations.Measurements
All from one real project of 177 tests that enables both
requireCoverageMetadataandbeStrictAboutCoverageMetadata.Recording by observation costs 2.6× on a run that was already collecting coverage, and all of it lands in the tests declaring
#[CoversNothing], which have to be collected for and are usually the slow ones. Deriving from declarations is free.For 165 of 165 tests that declare targets, the declarations covered everything the test was observed to execute — no exceptions beyond
#[CoversNothing]. The price is a coarser selection:Declaration is never below observation on any file, and costs between nothing and half as many tests again. Five of the 177 tests declare no targets, are never recorded in that mode, and therefore always run: that is the floor.
Selecting after editing two source files gave 26 of 177 whether the change set was worked out from the hashes or piped in from
git diff --name-only. With nothing edited,--only-impactedselected 0 of 177 in 0.14 seconds. The data is about 150 to 350 bytes per test — 27 KiB for this project.What you can rely on
Everything the analysis has no reliable information about causes the test to run:
#[CoversNothing]and a PHPT test#[Depends]And these cause every test to run:
--impacted-byor in a list, is not among the files that were recordedcomposer.lockchangedThe last one matters more than it sounds: what was recorded describes one state of the world, and when that state is not what it was, the honest answer is not that some entries are stale but that none of them can be trusted.
--only-impactedrefuses to run at all without a cache directory, or when the test run history is turned off, rather than quietly being less careful.Nothing is kept forever, either. A test run forgets every entry it did not record itself, but only when that run ran every test there is. A run that filtered, that selected by impact, or that stopped at the first failure recorded nothing for the tests it did not run, and nothing tells those apart from tests that are gone, so such a run adds to what is known and takes nothing away.
#[UsesFixture]Neither observation nor declaration can see a data file: reading
invoices.csvis not executing code, and a coverage target names code units. A new attribute closes that:It goes on a test class, a test method, or a data provider method: the last is the one that earns its keep, because a provider shared by many tests declares the file once and every test using it inherits the dependency. Directories work too, watched as a whole, so adding a file to one counts as a change. A path that does not exist produces a warning and is ignored, the same treatment a coverage target that cannot be used gets. Unlike coverage targets, nothing can ever verify this attribute; it is a promise the project makes to itself.
Decisions
#[CoversNothing]would otherwise get 2.6× slower unasked.#[CoversNothing]are collected for in observation mode, with the targets bypassed so nothing reaches the coverage report. The alternative — treating them as affected by everything — would put a floor under every selected run.<source>declarations, not the expanded file list. Hashing the list would throw everything away each time a class is added tosrc/, and buys nothing: a new file can only be reached if an existing one changed.--only-impacted,--impacted-byand--impacted-by-fileare command line options only. A switch that makes every run partial does not belong in a file a project commits.