Repository navigation
fix: isolate mid-sweep reload signal from the test runner - #1131
Open
rootsiemen wants to merge 1 commit into
Open
rootsiemen wants to merge 1 commit into
rootsiemen wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1126
Problem
When the polling watcher detects a change it calls
FileMonitorWatcher::reload()→Utils::reload(true), which sendsSIGUSR1 to the parent process. The mid-sweep fixture only ignored SIGUSR1
in itself, so the signal still hit the fixture's parent (PHPUnit, or any
shell running the fixture) and could terminate it with the default signal
action.
Fix
tests/Fixtures/polling_watcher_mid_sweep_runner.phpnow supervises thesweep in a forked child process:
from the watcher lands on the supervisor, never on the test runner.
SIG_IGNand runs the sweep unchanged.(exit 3) unless exactly one reload signal arrived — one file changes,
so one reload is expected. The existing exit-code contract (0/1/2/3)
is preserved.
Files changed
tests/Fixtures/polling_watcher_mid_sweep_runner.php— sweep body movedinto
runMidSweepCheck()(same checks,exit()→return), plus thefork supervisor with the reload-signal count check.
tests/PollingMonitorWatcherTest.php— newtestMidSweepReloadSignalDoesNotReachTestRunnerregression test: it runsthe fixture and asserts exit 0, which now also proves the parent
survived and the signal stayed inside the fixture.
Tests run
please rely on CI for the full suite, including the two existing
mid-sweep tests and the new regression test.