Skip to content

Commit cec6d25

Browse files
Testclaude
Test
and
claude
committed
test(memory-budget): end-to-end regression for BUG-001 and BUG-002
Adds two ci-features regression tests that exercise the memory-budget features through the real `php githooks` binary against a tmp config file. Closes the cycle on the two bugs detected during v3.3 RC QA: - bug001_per_job_fail_above_propagates_to_exit_code Pins the contract closed in b9f5ff8: a job whose peak crosses `memory.fail-above` must flip flow.success to false and exit non-zero. Pre-fix the JobResult had threshold.failed=true but job.success=true and exit code 0 — the regression was silent in CI. Verified by manually reverting the fix in FlowMemoryHandler (the threshold→success propagation block) and confirming the test fails with "exit code must be non-zero". - bug002_memory_warn_above_below_reserve_does_not_deadlock Pins the contract closed in d614e4c: when --memory-warn-above is set BELOW max(jobs.memory), the bin-packing reference must clamp individual reserves so AdmissionContext::fits() can still admit the head; otherwise the executeParallel loop spins at 100% CPU. The binary is wrapped in `timeout 30` and exit code 124 is the deadlock sentinel. Verified by reverting the clamp in FlowExecutor::buildProcessPool — the test fails in 30s with "executeParallel loop deadlocked". Lives under @group ci-features (not @group release) because invoking the source-checkout `githooks` script avoids the cost of rebuilding the .phar on every test run; the matrix CI cell already uses this path for the other memory-budget feature tests. The unit and integration tests at the FlowExecutor / FlowMemoryHandler level remain the first line of defence; these E2E tests are the safety net that catches any refactor or packaging regression that bypasses those layers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9f716bd commit cec6d25

1 file changed

Lines changed: 157 additions & 0 deletions

File tree

‎tests/System/CiFeatures/MemoryBudgetRegressionTest.php‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\System\CiFeatures;
6+
7+
use Tests\Utils\TestCase\CiFeatureTestCase;
8+
9+
/**
10+
* End-to-end regression tests for the two memory-budget bugs detected during
11+
* v3.3 RC QA and closed in commits b9f5ff8 (BUG-001) and d614e4c (BUG-002).
12+
*
13+
* Sits in the CiFeatures group so the binary is invoked from the source
14+
* checkout (no need for a freshly-built .phar) — the same path the matrix
15+
* CI cell uses for memory-budget feature tests. The unit and integration
16+
* tests on FlowExecutor / FlowMemoryHandler already cover the contracts at
17+
* the coordinator level; these tests are the last-line safety net against
18+
* a refactor that bypasses the unit tests yet ships a broken end-to-end
19+
* behaviour to users.
20+
*
21+
* @group ci-features
22+
*/
23+
class MemoryBudgetRegressionTest extends CiFeatureTestCase
24+
{
25+
private string $burnerScript;
26+
27+
protected function setUp(): void
28+
{
29+
parent::setUp();
30+
$this->burnerScript = 'tests/Fixtures/scripts/memory-burner.php';
31+
}
32+
33+
/**
34+
* Regression for BUG-001 (closed in b9f5ff8 — fix(memory): propagate per-job
35+
* fail-above to job.success).
36+
*
37+
* Symptom before the fix: a job whose observed peak crossed its declared
38+
* `memory.fail-above` produced `threshold.failed: true` on the JobResult,
39+
* but `job.success` stayed true, `flow.success` stayed true, and the exit
40+
* code was 0. The contract said a job that crossed fail-above must fail
41+
* the flow (mirror of the time-budget per-job `fail-after` contract).
42+
*
43+
* Setup: a 64 MB allocator (memory-burner.php) with warn-above=10 and
44+
* fail-above=20 — the observed peak (~64 MB) crosses both.
45+
*
46+
* @test
47+
*/
48+
public function bug001_per_job_fail_above_propagates_to_exit_code(): void
49+
{
50+
if (PHP_OS_FAMILY === 'Windows') {
51+
$this->markTestSkipped('Per-job memory threshold requires an active sampler; Windows skipped.');
52+
}
53+
54+
$this->configurationFileBuilder
55+
->setV3Flows(['qa' => ['jobs' => ['mem_heavy']]])
56+
->setV3Jobs([
57+
'mem_heavy' => [
58+
'type' => 'custom',
59+
'script' => "php $this->burnerScript 64 2",
60+
'memory' => ['warn-above' => 10, 'fail-above' => 20],
61+
],
62+
]);
63+
$this->writeConfig();
64+
65+
$result = $this->runGithooks("flow qa --format=json --config=$this->configPath");
66+
67+
$this->assertNotSame(
68+
0,
69+
$result['exitCode'],
70+
"BUG-001 regression: job that crossed fail-above must produce a non-zero exit code. stderr:\n{$result['stderr']}"
71+
);
72+
73+
$decoded = $this->decodeJsonOutput($result['stdout']);
74+
$this->assertFalse(
75+
$decoded['success'],
76+
'BUG-001 regression: flow.success must be false when a per-job fail-above triggers'
77+
);
78+
79+
$job = $this->findJob($decoded, 'mem_heavy');
80+
$this->assertFalse(
81+
$job['success'],
82+
'BUG-001 regression: job.success must be false when peak crossed fail-above (was true before b9f5ff8)'
83+
);
84+
$this->assertTrue(
85+
$job['memoryThreshold']['failed'] ?? false,
86+
'memoryThreshold.failed must be true so the threshold reason is observable in the structured output'
87+
);
88+
}
89+
90+
/**
91+
* Regression for BUG-002 (closed in d614e4c — fix(memory): clamp per-job
92+
* memory reserve to bin-packing reference in admission).
93+
*
94+
* Symptom before the fix: invoking `flow qa --memory-warn-above=N` with
95+
* N < max(jobs.memory) caused the executeParallel loop to spin at 100% CPU
96+
* forever. The bin-packing reference (warn-above preferred, fail-above
97+
* otherwise) was used as a hard admission ceiling, so a single job whose
98+
* declared `memory:` exceeded that ceiling never satisfied
99+
* AdmissionContext::fits() and FifoAdmission span the loop indefinitely.
100+
*
101+
* Wrapping the binary in `timeout 30` is critical: without it, a regression
102+
* of this fix would hang the test runner for PHPUnit's full wall-clock
103+
* budget. With it, an unfinished binary surfaces as exit code 124 — the
104+
* sentinel we assert against.
105+
*
106+
* @test
107+
*/
108+
public function bug002_memory_warn_above_below_reserve_does_not_deadlock(): void
109+
{
110+
$this->configurationFileBuilder
111+
->setV3GlobalOptions(['fail-fast' => false, 'processes' => 1])
112+
->setV3Flows(['qa' => ['jobs' => ['heavy_reservation']]])
113+
->setV3Jobs([
114+
'heavy_reservation' => [
115+
'type' => 'custom',
116+
'script' => 'true',
117+
'memory' => 300,
118+
],
119+
]);
120+
$this->writeConfig();
121+
122+
$entrypoint = $this->projectRoot . DIRECTORY_SEPARATOR . 'githooks';
123+
$cmd = sprintf(
124+
'timeout 30 %s %s flow qa --memory-warn-above=200 --format=json --config=%s 2>/dev/null',
125+
escapeshellarg(PHP_BINARY),
126+
escapeshellarg($entrypoint),
127+
escapeshellarg($this->configPath)
128+
);
129+
130+
$start = microtime(true);
131+
$output = shell_exec($cmd . '; echo "EXIT_CODE=$?"');
132+
$elapsed = microtime(true) - $start;
133+
134+
// shell_exec returns the combined stdout (with our marker on the last line).
135+
if (!is_string($output) || preg_match('/EXIT_CODE=(\d+)/', $output, $m) !== 1) {
136+
$this->fail('Could not extract exit code from binary output: ' . var_export($output, true));
137+
}
138+
$exitCode = (int) $m[1];
139+
$jsonOutput = preg_replace('/EXIT_CODE=\d+\s*$/', '', $output);
140+
141+
$this->assertNotSame(
142+
124,
143+
$exitCode,
144+
'BUG-002 regression: the executeParallel loop deadlocked — `timeout 30` killed the binary at the 30s sentinel'
145+
);
146+
$this->assertLessThan(
147+
10.0,
148+
$elapsed,
149+
'BUG-002 regression: flow took ' . number_format($elapsed, 2) . 's. '
150+
. 'After the fix the binary terminates in <2s; >10s indicates the clamp is missing or partially regressed'
151+
);
152+
153+
$decoded = json_decode($jsonOutput, true);
154+
$this->assertNotNull($decoded, 'Output is not valid JSON: ' . $jsonOutput);
155+
$this->assertSame('qa', $decoded['flow']);
156+
}
157+
}

0 commit comments

Comments
 (0)