fix(launch_manager): enable ASSERT_DBG in integration tests - #591
Draft
hskang-amelia wants to merge 3 commits into
Draft
fix(launch_manager): enable ASSERT_DBG in integration tests#591hskang-amelia wants to merge 3 commits into
hskang-amelia wants to merge 3 commits into
Conversation
Follow-up to eclipse-score#579, which wired SCORE_LANGUAGE_FUTURECPP_ASSERT_LEVEL_DEBUG into unit tests via lm_cc_test but left integration tests untouched. Integration tests link the production launch_manager daemon binary directly, so enabling the define there would mean enabling it on the target that ships in production. Instead, split the daemon's cc_binary into a launch_manager_binary macro (score/launch_manager/src/daemon/daemon.bzl) so a second target, launch_manager_debug_asserts, can share the same srcs/deps but add the debug-assert define, leaving the production launch_manager target untouched. Each tests/integration/*/BUILD now points its `binaries` list at the debug variant. integration_test() gains a binary_renames option (backed by pkg_files's renames) so the packaged binary still shows up as `launch_manager` in the test environment — the test scripts hardcode that filename, and the debug-variant target has a different Bazel target name. Test plan: - bazel build --lockfile_mode=error --config=x86_64-linux //score/launch_manager/src/daemon/... - bazel test --lockfile_mode=error --config=x86_64-linux //score/launch_manager/... (16/16 executed unit tests pass) - bazel run //:format.check_Starlark_with_buildifier - Ran the 19 integration tests locally in host mode with the debug-assert variant; failures observed in this environment reproduce identically against the unmodified production binary (signal-handling and a Rust-toolchain GLIBC mismatch, both pre-existing and unrelated to this change).
LocalAsyncProcess.stop() sent SIGTERM to the whole process group so that the actual test binary (launched under fakeroot) would receive it and run its cleanup code. But fakeroot's /bin/sh wrapper traps EXIT/INT, not TERM, so the broadcast killed the wrapper itself before it could wait() for its child and relay the real exit code -- the tracked returncode then reported the wrapper's own signal death (-15) regardless of whether the wrapped binary actually shut down cleanly. Signal only the process(es) fakeroot launched instead, and let the wrapper's shell script finish normally and propagate the real exit status. Test plan: - Ran the launch_manager integration suite locally in host mode (--//config:integration_mode=host): all 17 tests unaffected by pre-existing local environment limitations (a missing SCHED_FIFO capability, and a GLIBC/rustc mismatch pulled in by an unrelated Rust dependency) now pass and correctly report the wrapped binary's own exit status.
hskang-amelia
requested a deployment
to
workflow-approval
September 4, 2026 04:07 — with
GitHub Actions
Waiting
hskang-amelia
requested a deployment
to
workflow-approval
September 4, 2026 04:07 — with
GitHub Actions
Waiting
3 tasks
Contributor
|
Documentation preview for this pull request is available at: |
hskang-amelia
requested a deployment
to
workflow-approval
September 4, 2026 04:46 — with
GitHub Actions
Waiting
hskang-amelia
requested a deployment
to
workflow-approval
September 4, 2026 04:46 — with
GitHub Actions
Waiting
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.
Summary
Follow-up to #579 (comment: #579 (comment)), which wired
SCORE_LANGUAGE_FUTURECPP_ASSERT_LEVEL_DEBUGinto unit tests vialm_cc_testbut left integration tests untouched.Integration tests link the production
launch_managerdaemon binary directly, so enabling the define there would mean enabling it on the same target that ships in production. Instead:cc_binaryinto alaunch_manager_binarymacro (score/launch_manager/src/daemon/daemon.bzl) so a second target,launch_manager_debug_asserts, can share the same srcs/deps but add the debug-assert define, leaving the productionlaunch_managertarget untouched.binarieslist in eachtests/integration/*/BUILDat the debug variant instead.binary_renamesoption tointegration_test()(backed bypkg_files'srenames) so the packaged binary still shows up aslaunch_managerin the test environment — the test scripts hardcode that filename, and the debug-variant target has a different Bazel target name.While verifying this locally in host mode, also found and fixed a pre-existing bug in
tests/utils/plugins/localhost.py:LocalAsyncProcess.stop()sentSIGTERMto the whole process group so children launched underfakerootwould receive it, butfakeroot's/bin/shwrapper trapsEXIT/INT, notTERM— so the broadcast killed the wrapper itself before it couldwait()for its child and relay the real exit code, making every host-mode integration test reportLCM did not exit cleanly, it died with code -15regardless of whether the daemon actually shut down cleanly. Fixed by signaling only the process(es)fakerootlaunched.Test plan
bazel build --lockfile_mode=error --config=x86_64-linux //score/launch_manager/src/daemon/...bazel test --lockfile_mode=error --config=x86_64-linux //score/launch_manager/...(16/16 executed unit tests pass)bazel run //:format.check_Starlark_with_buildifier--//config:integration_mode=host: 17/19 pass in an ad-hoc sandbox, and confirmed the remaining 2 are environment limitations rather than caused by this change:complex_monitoringfailed to build in the ad-hoc sandbox (Ubuntu, GLIBC 2.31): itscomponent_complex_monitoringhelper depends on//score/health_monitor's Rust FFI, which needs a newer GLIBC than that sandbox's rustc/ferrocene toolchain supports. Confirmed fixed by building the same target inside the repo's ownghcr.io/eclipse-score/devcontainer:v1.10.0image (Ubuntu 24.04, GLIBC 2.39) — it builds and runs cleanly there.sandbox_optionsfails in its ownrequire_realtime_schedulingfixture (before the daemon binary even runs):chrt: failed to set pid 0's policy: Operation not permitted. This one is not resolved by the devcontainer alone: even insideghcr.io/eclipse-score/devcontainer:v1.10.0with the--cap-add=SYS_NICEfrom.devcontainer/devcontainer.json'srunArgs, it still fails on a Docker host whose daemon hasn't been separately configured with real-time bandwidth — the container'scpu.rt_runtime_uscgroup value is0unless the host's/etc/docker/daemon.jsonsets{"cpu-rt-period": 1000000, "cpu-rt-runtime": 950000}and the daemon is restarted (exactly what the test's own failure message says), which is a host-level setting outside this repo's control.