From 3dc7d48343a2197bef409c8b5cc5063c1f030843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Thu, 29 Oct 2026 23:06:04 +0100 Subject: [PATCH] fix: make polling CI checks order independent (#1122) --- .github/workflows/tests.yaml | 1 + tests/CoverageCiGateTest.php | 17 +++++ .../polling_watcher_mid_sweep_runner.php | 67 +++++++++++++------ tests/PollingMonitorWatcherTest.php | 13 ++++ 4 files changed, 77 insertions(+), 21 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 3e2a9f05..e8c8c8f6 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -138,6 +138,7 @@ jobs: run: composer coverage:check - name: Sanitize artifact name + if: always() id: artifact run: | name="coverage-${{ matrix.php-version }}-${{ matrix.symfony-version }}" diff --git a/tests/CoverageCiGateTest.php b/tests/CoverageCiGateTest.php index 9121c704..0722544c 100644 --- a/tests/CoverageCiGateTest.php +++ b/tests/CoverageCiGateTest.php @@ -44,6 +44,23 @@ public function testCoverageReportUploadRunsEvenWhenGateFails(): void ); } + public function testArtifactNameIsAvailableWhenTestsOrCoverageGateFail(): void + { + $workflow = self::readWorkflow(); + + self::assertMatchesRegularExpression( + '/- name: Sanitize artifact name(?:(?!- name:).)*if: always\(\)(?:(?!- name:).)*id: artifact(?:(?!- name:).)*GITHUB_OUTPUT/s', + $workflow, + 'The artifact name step must run even after a failure and publish its output', + ); + self::assertStringContainsString('name="coverage-${{ matrix.php-version }}-${{ matrix.symfony-version }}"', $workflow); + self::assertStringContainsString('name="${name//\\*/x}"', $workflow); + self::assertMatchesRegularExpression( + '/- name: Upload coverage report(?:(?!- name:).)*name: \${{ steps\.artifact\.outputs\.name }}/s', + $workflow, + ); + } + public function testThresholdIsDefinedOnlyInComposerScript(): void { $workflow = self::readWorkflow(); diff --git a/tests/Fixtures/polling_watcher_mid_sweep_runner.php b/tests/Fixtures/polling_watcher_mid_sweep_runner.php index 95733450..939278ec 100644 --- a/tests/Fixtures/polling_watcher_mid_sweep_runner.php +++ b/tests/Fixtures/polling_watcher_mid_sweep_runner.php @@ -7,7 +7,7 @@ * (the prefix of the tree that was visited in earlier ticks) is detected * on the next sweep. * - * Usage: php polling_watcher_mid_sweep_runner.php + * Usage: php polling_watcher_mid_sweep_runner.php [tail] * * Exit codes: * 0 — success (detection occurred on the next sweep) @@ -19,6 +19,7 @@ $tempDir = $argv[1] ?? ''; $autoloadPath = $argv[2] ?? ''; +$tail = ($argv[3] ?? '') === 'tail'; if ($tempDir === '' || $autoloadPath === '' || !is_dir($tempDir)) { fprintf(STDERR, "Usage: php polling_watcher_mid_sweep_runner.php \n"); @@ -76,39 +77,63 @@ $checkMethod = $watcherClass->getMethod('checkFileSystemChanges'); -// Tick 1: processes first 500 files. -$checkMethod->invoke($watcher); +// Select from the actual traversal order, not from file names or creation order. +$entries = iterator_to_array(new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($tempDir, FilesystemIterator::SKIP_DOTS | FilesystemIterator::UNIX_PATHS), + RecursiveIteratorIterator::LEAVES_ONLY, +), false); +$target = $entries[$tail ? 599 : 0]->getPathname(); +$iteratorsProp = $watcherClass->getProperty('iterators'); -// Tick 2: completes the sweep (remaining 100 files). +// Tick 1 visits the prefix and leaves the sweep in progress. $checkMethod->invoke($watcher); - -// Verify the sweep completed (no resume state). -$iteratorsProp = $watcherClass->getProperty('iterators'); $iterators = $iteratorsProp->getValue($watcher); -if (!empty($iterators)) { - fprintf(STDERR, "FAIL: Sweep did not complete after 2 ticks (iterators not empty)\n"); +if (empty($iterators)) { + fprintf(STDERR, "FAIL: Sweep completed before the second tick\n"); exit(3); } -// Now modify file0.php (in the already-scanned prefix). -// Ensure the mtime is strictly greater than lastMTime. -sleep(1); -file_put_contents($tempDir . '/file0.php', 'invoke($watcher); +} -$modifiedMTime = filemtime($tempDir . '/file0.php'); +// In the prefix case this modification happens between ticks, after the +// watcher visited the file. The mtime must exceed its whole-second watermark. +sleep(1); +file_put_contents($target, 'invoke($watcher); - -$afterMTime = $lastMTimeProp->getValue($watcher); +if (!$tail) { + // The remaining part of the old sweep must not revisit the prefix. + $checkMethod->invoke($watcher); + if ($lastMTimeProp->getValue($watcher) !== $initialMTime) { + fprintf(STDERR, "FAIL: Modified prefix was detected before the next sweep\n"); + exit(2); + } +} +$iterators = $iteratorsProp->getValue($watcher); +if (!empty($iterators)) { + fprintf(STDERR, "FAIL: Sweep did not complete after 2 ticks\n"); + exit(3); +} -if ($afterMTime <= $initialMTime) { - fprintf(STDERR, "FAIL: lastMTime not updated after modifying a file in the already-scanned region (was %d, now %d)\n", $initialMTime, $afterMTime); +// A whole new sweep takes two ticks; the changed file can be in either one. +$checkMethod->invoke($watcher); +if ($tail && $lastMTimeProp->getValue($watcher) !== $initialMTime) { + fprintf(STDERR, "FAIL: Tail file was detected in the first tick\n"); + exit(2); +} +if ($lastMTimeProp->getValue($watcher) === $initialMTime) { + $checkMethod->invoke($watcher); +} +if ($lastMTimeProp->getValue($watcher) !== $modifiedMTime) { + fprintf(STDERR, "FAIL: Modified file was not detected during the next sweep\n"); exit(2); } diff --git a/tests/PollingMonitorWatcherTest.php b/tests/PollingMonitorWatcherTest.php index 17f978c8..34808a45 100644 --- a/tests/PollingMonitorWatcherTest.php +++ b/tests/PollingMonitorWatcherTest.php @@ -590,6 +590,19 @@ public function testFileModifiedInAlreadyScannedRegionTriggersReloadOnNextSweep( ); } + public function testFileAtEndOfNextSweepIsDetectedOnSecondTick(): void + { + $autoloadPath = \realpath(__DIR__ . '/../vendor/autoload.php'); + if ($autoloadPath === false) { + self::markTestSkipped('vendor/autoload.php not found.'); + } + + $scriptFile = __DIR__ . '/Fixtures/polling_watcher_mid_sweep_runner.php'; + $exitCode = $this->runPhpScript($scriptFile, [$this->tempDir, $autoloadPath, 'tail']); + + self::assertSame(0, $exitCode, 'A file at the end of a sweep should be detected on its second tick.'); + } + /** * Adding or removing a directory between ticks must not throw, and * the sweep must still converge. APFS readdir order is hash-based (not