fix(ci): resolve Windows test failures and stop swallowing test errors#242
Closed
kcenon wants to merge 1 commit into
Closed
fix(ci): resolve Windows test failures and stop swallowing test errors#242kcenon wants to merge 1 commit into
kcenon wants to merge 1 commit into
Conversation
The Windows CI has been failing due to monitoring_system dependency tests (fixed upstream in monitoring_system PRs #610 and #611). This CI run will pick up those fixes since dependencies are cloned from HEAD. Additionally, fix test steps that were silently swallowing ctest failures via `|| echo "Some tests failed"` on Unix and a no-op `Write-Host` on Windows. Test failures now properly fail the CI job. Add dependency version logging to make it easier to diagnose which upstream commit was used when dependency test failures occur.
Owner
Author
|
Closing this PR. The "stop swallowing test errors" changes exposed many pre-existing test failures across all platforms that are beyond the scope of the Windows monitoring_system fix. A new, targeted PR will be created. |
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
1 participant
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.
What
Fix Windows CI test failures caused by upstream monitoring_system dependency tests, and
correct test error handling across all platforms to prevent silent test failures.
Change Type
Affected Components
.github/workflows/ci.yml- CI workflow test execution and dependency managementWhy
Problem Solved
PRs #239 and #241 were both merged with Windows CI failures (3 monitoring_system tests
failing on Windows:
SystemResourceCollectorTest.ContextSwitchMonitoring,MetricsProviderTest.TemperatureAvailabilityCheck,StressPerformanceTest.HighLoadStressTest).These failures originated from the monitoring_system dependency, which has since been fixed
upstream (monitoring_system PRs #610 and #611, both merged to main). Since this CI clones
dependencies from HEAD via
git clone --depth 1, this PR's CI run will pick up the fixes.Additionally, test failures were being silently swallowed on all platforms:
ctest ... || echo "Some tests failed"always exits 0if ($LASTEXITCODE -ne 0) { Write-Host "..." }exits 0 from Write-HostRelated Issues
Where
Files Changed
.github/workflows/ci.ymlHow
Changes Made
Remove error-swallowing patterns: Remove
|| echo "Some tests failed"from Unixtest steps and fix Windows PowerShell to explicitly
exit $LASTEXITCODEon failure.Test failures now properly fail the CI job on all platforms.
Add dependency version logging: New "Log dependency versions" steps (Unix and Windows)
that print the exact commit SHA of each cloned dependency. This makes it easy to diagnose
whether a dependency test failure is due to an outdated upstream clone.
Document dependency cloning behavior: Add comments explaining that dependencies are
cloned from HEAD of each repo's default branch, and that upstream fixes must be merged
before they take effect in this CI.
Testing
includes the Windows test fixes from PRs #610 and #611)
CI run, and the remaining 3 are fixed by the latest monitoring_system HEAD)