Add split shell PMaaS integtests - #1001
Conversation
5a9fa23 to
5271327
Compare
|
Ooh stacks exist! This is a feature I did not know I want, but I've been working this way for a loongg time. Lets try it here :) |
67fe207 to
cb80f08
Compare
emmuhamm
left a comment
There was a problem hiding this comment.
Hi @PawelPlesniak, this is ready for a look on your end now. I wouldn't say review since it's still waiting on Kurt's developments, but hey we can build on it already :)
I've left a few comments on things that I thought about and didnt have enough time, or points of interests as usual.
However, in terms of the future I would say that these set of tests is enough to test PMaaS for now, and should be fine to close the PMaaS issue. Of course, we can expand these in the testing framework later on but this might be good for now.
Let me know what you think, and feel free to tag Kurt as well if you think this is mature enough.
| # 05-Aug-2026, KAB: the goal of this test is to validate and demonstrate the use of multiple | ||
| # user-specified applications running in the DAQ session that is part of this test. | ||
| # | ||
| # This integtest was created by copying the small_footprint_quick_test from the daqsystemtest | ||
| # repo and converting the assignment of the run control commands to make use of the new | ||
| # "daq_session_ingredients" special integtest variable. |
There was a problem hiding this comment.
Ah forgot to update the top of the files but I've run out of time :/
There was a problem hiding this comment.
There should be an issue tackled on cleaning up integ_test_utils, I think its gonna get a little bit webby and tangly
| echo pre_boot_2 | ||
| echo-on-server pre_boot_2 | ||
| ps -w 180 | ||
| boot config/daqsystemtest/example-configs.data.xml local-1x1-config {daq_session_name_1} |
There was a problem hiding this comment.
as mentioned in the description, we are currently unable to do another boot config/daqsystemtest/example-configs.data.xml local-1x1-config {daq_session_name} because when you do this it comes out with a 'theres another session already booted, are you sure? Y/N`.
Haven't had the time to figure out what to do about this yet so I've left it
| pm_connect = "connected from process_manager_shell" | ||
| pms_connect = "connected to the process manager through a" |
There was a problem hiding this comment.
Bit of a broad test, maybe if we want to we can also do a 'check if we have the right user' and 'check if its connecting to the right places (ports) and stuff.
| assert_rows_have_valid_uuids(ps_post_boot) | ||
|
|
||
|
|
||
| def test_boot_pm(run_dunerc) -> None: |
There was a problem hiding this comment.
Maybe combine the test_boots into one test?
| ) | ||
|
|
||
|
|
||
| def test_restart_mlt_logs_pms(run_dunerc) -> None: |
There was a problem hiding this comment.
very light test but does the job, but I wonder if theres other things we can add on this test?
| # TODO: This bit here is grabbing functions from the integ test utils. Maybe it can be better optimised? | ||
| table_start_idx = find_line_index( | ||
| lines_pms, | ||
| lambda line: "Terminated process" in line, | ||
| start_idx=pre_boot_idx_pms + 1, | ||
| ) | ||
|
|
||
| assert table_start_idx is not None, "cannot fine terminated process table" | ||
|
|
||
| terminated_table = _parse_table_from_index(lines_pm, table_start_idx, _PS_COLUMNS) | ||
| for row in terminated_table: | ||
| assert UUID_RE.match(row["uuid"]), ( | ||
| f"Expected a valid UUID for process '{row['friendly_name']}', got '{row['uuid']}'" | ||
| ) |
There was a problem hiding this comment.
See the comment here. Maybe this would be better left as a function in integ_test_utils?
| pm_port = find_free_port(50020, 52000) | ||
|
|
||
| # The command lines that should be used to start the applications | ||
| procmsg_startup_commands = ["drunc-process-manager", "<proc_mgr_choice>", str(pm_port)] |
There was a problem hiding this comment.
Something to keep in mind that so far all of this is near hardcoded to use the ssh-standalone thing.
We should absolutely make tests for cern kafka as well and later on k8s. maybe in the next PR? and maybe this is beyond the scope of PMaaS but something for the testing framework?
Description
Builds on #997
New tests
Adds three tests:
Relevant developments
Several developments need to happen in order for this to work
echo-on-serverto the process managerdrunc.echoinstead ofself.logecho-on-serverto the unified shell as wellDependencies
Some dependencies as well
emmuhamm/echo-on-serverdruncschema (No PR yet)Testing
Important note: This is currently targetting kurt's branch, which is out of date with develop. To test this, this branch will need to be build on top of kurt's branch, which has to also be built on top of an up to date develop.
emmuhamm/split-shell-tests-correct-dependenciesto test if the thingys workpytest -s --tb=short /nfs/home/emmuhamm/nightly/NFD_DEV_260807_A9/pythoncode/drunc/src/drunc/integtest/pm*.py --integtest-verbosity 5Tested on np04-srv-028 on 07 aug 26 nightly, all integtests (except failure_.py) passes. Haven't testred on failure_.py due to issues reported by Kurt
Ruff and pytest and msqt and stuff haven't been done since still waiting on Kurts stuff. MSQT passes though
Developer checklist
Prior to marking this as "Ready for Review"
Tests ran on: WHAT HOSTNAME from release RELEASE_NAME
Unit tests - some tests can't be ran on the CI. This is documented. If this PR checks a feature that can't be tested with CI, this has been marked appropriately.
Integration tests - the
daqsystemtest_integtest_bundlerequires a lot of resources, and connections to the EHN1 infrastructure. Check the cross referenced list if you can't run these. The developer needs to run at least the .pytest --marker) passed_INSERT MAKER NAME HERE_daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.pydaqsystemtest_integtest_bundle.shdunedaq_integtest_bundle.sh -r drunc)Final checklist prior to marking this as "Ready for Review"
Reviewer checklist
src/daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.pyifdaqsystemtest_integtest_bundle.shdruncare in the log filesdruncfailure appears:scripts/drunc_integtest_bundle.sh)Once the above boxes are checked, the PR(s) can be merged following the steps below.
Choose one of the following an complete all substepsPrior to merging
Once completed, the reviewer can merge the PR.
Notification message for a Slack channel
Note - this should be to #dunedaq-integration for general workflow that isn't during a release candidate period, and to #daq-release-prep otherwise.
For an single merge that changes the user workflow
For co-ordinated merge