diff --git a/app/Jobs/PushServerUpdateJob.php b/app/Jobs/PushServerUpdateJob.php index 7726c2c73..32baf3f07 100644 --- a/app/Jobs/PushServerUpdateJob.php +++ b/app/Jobs/PushServerUpdateJob.php @@ -269,6 +269,7 @@ private function aggregateMultiContainerStatuses() // Aggregate status: if any container is running, app is running $hasRunning = false; $hasUnhealthy = false; + $hasUnknown = false; foreach ($relevantStatuses as $status) { if (str($status)->contains('running')) { @@ -276,12 +277,21 @@ private function aggregateMultiContainerStatuses() if (str($status)->contains('unhealthy')) { $hasUnhealthy = true; } + if (str($status)->contains('unknown')) { + $hasUnknown = true; + } } } $aggregatedStatus = null; if ($hasRunning) { - $aggregatedStatus = $hasUnhealthy ? 'running (unhealthy)' : 'running (healthy)'; + if ($hasUnhealthy) { + $aggregatedStatus = 'running (unhealthy)'; + } elseif ($hasUnknown) { + $aggregatedStatus = 'running (unknown)'; + } else { + $aggregatedStatus = 'running (healthy)'; + } } else { // All containers are exited $aggregatedStatus = 'exited (unhealthy)'; diff --git a/tests/Unit/ExcludeFromHealthCheckTest.php b/tests/Unit/ExcludeFromHealthCheckTest.php index 2c58b0caa..07d6791de 100644 --- a/tests/Unit/ExcludeFromHealthCheckTest.php +++ b/tests/Unit/ExcludeFromHealthCheckTest.php @@ -19,7 +19,9 @@ ->toContain('// If all containers are excluded, calculate status from excluded containers') ->toContain('// but mark it with :excluded to indicate monitoring is disabled') ->toContain('if ($relevantContainerCount === 0) {') - ->toContain("return 'running:excluded';") + ->toContain("return 'running:unhealthy:excluded';") + ->toContain("return 'running:unknown:excluded';") + ->toContain("return 'running:healthy:excluded';") ->toContain("return 'degraded:excluded';") ->toContain("return 'exited:excluded';"); }); diff --git a/tests/Unit/PushServerUpdateJobStatusAggregationTest.php b/tests/Unit/PushServerUpdateJobStatusAggregationTest.php new file mode 100644 index 000000000..e1ef73087 --- /dev/null +++ b/tests/Unit/PushServerUpdateJobStatusAggregationTest.php @@ -0,0 +1,107 @@ + unknown > healthy + * + * This ensures consistency with GetContainersStatus::aggregateApplicationStatus() + * and prevents the bug where "unknown" status was incorrectly converted to "healthy". + */ +it('aggregates status with unknown health state correctly', function () { + $jobFile = file_get_contents(__DIR__.'/../../app/Jobs/PushServerUpdateJob.php'); + + // Verify that hasUnknown tracking variable exists + expect($jobFile) + ->toContain('$hasUnknown = false;') + ->toContain('if (str($status)->contains(\'unknown\')) {') + ->toContain('$hasUnknown = true;'); + + // Verify 3-way status priority logic (unhealthy > unknown > healthy) + expect($jobFile) + ->toContain('if ($hasUnhealthy) {') + ->toContain('$aggregatedStatus = \'running (unhealthy)\';') + ->toContain('} elseif ($hasUnknown) {') + ->toContain('$aggregatedStatus = \'running (unknown)\';') + ->toContain('} else {') + ->toContain('$aggregatedStatus = \'running (healthy)\';'); +}); + +it('checks for unknown status alongside unhealthy status', function () { + $jobFile = file_get_contents(__DIR__.'/../../app/Jobs/PushServerUpdateJob.php'); + + // Verify unknown check is placed alongside unhealthy check + expect($jobFile) + ->toContain('if (str($status)->contains(\'unhealthy\')) {') + ->toContain('if (str($status)->contains(\'unknown\')) {'); +}); + +it('follows same priority as GetContainersStatus', function () { + $jobFile = file_get_contents(__DIR__.'/../../app/Jobs/PushServerUpdateJob.php'); + $getContainersFile = file_get_contents(__DIR__.'/../../app/Actions/Docker/GetContainersStatus.php'); + + // Both should track hasUnknown + expect($jobFile)->toContain('$hasUnknown = false;'); + expect($getContainersFile)->toContain('$hasUnknown = false;'); + + // Both should check for 'unknown' in status strings + expect($jobFile)->toContain('if (str($status)->contains(\'unknown\')) {'); + expect($getContainersFile)->toContain('if (str($status)->contains(\'unknown\')) {'); + + // Both should prioritize unhealthy over unknown over healthy + expect($jobFile)->toContain('} elseif ($hasUnknown) {'); + expect($getContainersFile)->toContain('} elseif ($hasUnknown) {'); +}); + +it('does not default unknown to healthy status', function () { + $jobFile = file_get_contents(__DIR__.'/../../app/Jobs/PushServerUpdateJob.php'); + + // The old buggy code was: + // $aggregatedStatus = $hasUnhealthy ? 'running (unhealthy)' : 'running (healthy)'; + // This would make unknown -> healthy + + // Verify we're NOT using ternary operator for status assignment + expect($jobFile) + ->not->toContain('$aggregatedStatus = $hasUnhealthy ? \'running (unhealthy)\' : \'running (healthy)\';'); + + // Verify we ARE using if-elseif-else with proper unknown handling + expect($jobFile) + ->toContain('if ($hasUnhealthy) {') + ->toContain('} elseif ($hasUnknown) {') + ->toContain('$aggregatedStatus = \'running (unknown)\';'); +}); + +it('initializes all required status tracking variables', function () { + $jobFile = file_get_contents(__DIR__.'/../../app/Jobs/PushServerUpdateJob.php'); + + // Verify all three tracking variables are initialized together + $pattern = '/\$hasRunning\s*=\s*false;\s*\$hasUnhealthy\s*=\s*false;\s*\$hasUnknown\s*=\s*false;/s'; + + expect(preg_match($pattern, $jobFile))->toBe(1, + 'All status tracking variables ($hasRunning, $hasUnhealthy, $hasUnknown) should be initialized together'); +}); + +it('preserves unknown status through sentinel updates', function () { + $jobFile = file_get_contents(__DIR__.'/../../app/Jobs/PushServerUpdateJob.php'); + + // The critical path: when a status contains 'running' AND 'unknown', + // both flags should be set + expect($jobFile) + ->toContain('if (str($status)->contains(\'running\')) {') + ->toContain('$hasRunning = true;') + ->toContain('if (str($status)->contains(\'unhealthy\')) {') + ->toContain('$hasUnhealthy = true;') + ->toContain('if (str($status)->contains(\'unknown\')) {') + ->toContain('$hasUnknown = true;'); + + // And then unknown should have priority over healthy in aggregation + expect($jobFile) + ->toContain('} elseif ($hasUnknown) {') + ->toContain('$aggregatedStatus = \'running (unknown)\';'); +});