Support deployment_config/working_dir value - #356
Conversation
GitHub Actions
Waiting
GitHub Actions
Waiting
License Check Results🚀 The license check job ran with the Bazel command: bazel run --lockfile_mode=error //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
GitHub Actions
Waiting
GitHub Actions
Waiting
GitHub Actions
Waiting
GitHub Actions
Waiting
GitHub Actions
Waiting
GitHub Actions
Waiting
| @@ -35,6 +36,8 @@ def run_until_file_deployed( | |||
| :param file_path: path of the file to wait for on the target. | |||
| :param timeout_s: maximum seconds to wait for the file (default: 30). | |||
| :param poll_interval_s: seconds between file-existence checks (default: 0.5). | |||
There was a problem hiding this comment.
Maybe update the documentation that poll_interval_s is also used to poll process state
There was a problem hiding this comment.
I think now after your latest rebase its no longer used for checking process state.
So now the documentation update is no longer correct
| TEST_STEP("Activate RunTarget Startup") | ||
| { | ||
| score::cpp::stop_token stop_token; | ||
| auto result = client.ActivateRunTarget("Startup").Get(stop_token); |
There was a problem hiding this comment.
Launch Manager goes to Startup by default. I think this is redundant.
Looks like we can remove the whole control_daemon_mock from this test
There was a problem hiding this comment.
As this test now uses two processes, I would rather stick to control_daemon_mock, because the control_daemon_mock is writing the test_end file. If we drop that process it is unclear when this test is finished, as now the two processes under test need to write the file instead.
There was a problem hiding this comment.
Think this is actually causing the error in https://github.com/eclipse-score/lifecycle/actions/runs/30273517385/job/90001693926?pr=356#step:4:1286
There was a problem hiding this comment.
Most probably this was related to the missing "is_self_terminating": true option in the application_profile.
It is also passing now in the pipeline: https://github.com/eclipse-score/lifecycle/actions/runs/30453507338/job/90581144811#step:4:4319
There was a problem hiding this comment.
Why is the separate Running and Off RunTarget needed for the test?
Can we not just put these processes into Startup and remove the control_daemon_mock altogether?
There was a problem hiding this comment.
Moving the processes to Startup state would remove the explicit EXPECT for the run target transition ( ActivateRunTarget("Running") ) from the test, which I don't like. If something is wrong with the run target under test, I do not want to dig in the production code, instead a simple assertion should fail.
Furthermore who writes then the test_end file on which run_until_file_deployed is waiting?
Of course run_until_file_deployed(..) could also wait on the list of files which are used in assert_test_results(..), but then you are no longer verifying that all children processes have finished immediately after switching to "Off".
For both reasons I would like to keep it as it is.
What I could do later on (on the other branch): Get rid of the test specific control_daemon_mock.cpp (for most tests) and create a common version in test_helper subfolder to reduce the duplication.
dad60c7 to
71264de
Compare
GitHub Actions
Waiting
Instead of hardcoded path in python.
sandbox_options_process_b was stuck in rare cases, as it never got cpu time and ran into timeout. This was caused by the minimal priority. So priority has been added here. Furthermore process_c added to check default options.
process_c configures neither a component-level working_dir nor one in the defaults section, so the launch manager falls back to bin_dir. Pass the expected default via --working-dir so the test verifies the default working directory path, mirroring how process_a verifies an explicit one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
144bfd4 to
d3e845a
Compare
GitHub Actions
Waiting
GitHub Actions
Waiting
Before, this warning appeared: Test execution time ... outside of range for MODERATE tests. Consider setting timeout="short" or size="small".
GitHub Actions
Inactive
GitHub Actions
Inactive
|
New test fails when run in devcontainer @TimoSteuerwaldETAS Can you modify the devcontainer json to make sure this works? |
| "//tests/utils/test_helper", | ||
| "@googletest//:gtest_main", | ||
| ], | ||
| ) |
There was a problem hiding this comment.
Suggestion:
Replace
cc_binary(
name = "sandbox_options_process_a",
srcs = [
"sandbox_options_process.cpp",
"verify_sandbox.hpp",
],
deps = [
"//score/launch_manager:control_cc",
"//score/launch_manager:lifecycle_cc",
"//tests/utils/test_helper",
"@googletest//:gtest_main",
],
)
cc_binary(
name = "sandbox_options_process_b",
srcs = [
"sandbox_options_process.cpp",
"verify_sandbox.hpp",
],
deps = [
"//score/launch_manager:control_cc",
"//score/launch_manager:lifecycle_cc",
"//tests/utils/test_helper",
"@googletest//:gtest_main",
],
)
cc_binary(
name = "sandbox_options_process_c",
srcs = [
"sandbox_options_process.cpp",
"verify_sandbox.hpp",
],
deps = [
"//score/launch_manager:control_cc",
"//score/launch_manager:lifecycle_cc",
"//tests/utils/test_helper",
"@googletest//:gtest_main",
],
)with
[
cc_binary(
name = "sandbox_options_process_{}".format(suffix),
srcs = [
"sandbox_options_process.cpp",
"verify_sandbox.hpp",
],
deps = [
"//score/launch_manager:control_cc",
"//score/launch_manager:lifecycle_cc",
"//tests/utils/test_helper",
"@googletest//:gtest_main",
],
)
for suffix in ("a", "b", "c")
]There was a problem hiding this comment.
Good idea! 🚀 Thanks for pointing out!
GitHub Actions
Waiting
GitHub Actions
Waiting
@NicolasFussberger I tried, for proxy reasons I am unable to use devcontainer. So I can not verify it. Please give it a try. |
3 participants
This fixes #343 .
Furthermore the config mapping tests have been fixed and are executed on the pipeline.
A feature integration test has been also added which is testing various options, not only working dir.