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.
There was a problem hiding this comment.
We can merge it for now, but I think we need to keep these tests minimal for what they testing. E.g. the control client API will be heavily changed and we need to update this test, even though the test scope has nothing to do with ControlClient or RunTarget transitions.
explicit EXPECT for the run target transition
You will still only see an assertion failure for a transition to Running. You will anyway need to dig through the config and then the logs to see why transition has failed. I do not see a huge benefit here.
who writes then the test_end file
I would just create a touch command as a component that depends on the sandbox processes. Then the file is only written if the sandbox processes were launched successfully.
The Off state is also not needed in the config or the explicit transition to Off state.
When launch_manager receives a SIGTERM it will terminate the processes.
dad60c7 to
71264de
Compare
GitHub Actions
Waiting
if process does not react on SIGTERM
- add another process - parameterize sandbox_options_process.cpp
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>
Before, this warning appeared: Test execution time ... outside of range for MODERATE tests. Consider setting timeout="short" or size="small".
Resp. remove outdated file names
Proxy was missing during sudo apt commands. Now config file for apt is written, which uses the content of the environment variable $http_proxy
Instead provide a way to keep environment variables needed on run time private.
To be used also to signal test_end.
Currently not passing.
This reverts commit 9e44e31.
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.