Fix integ test#933
Conversation
|
Hi @Aurashk, @PawelPlesniak, this is a small PR that fixes the PM integtest we have in drunc. See details in the issue. I think the changes in the output messages are from #896, so @Aurashk could you also take a look at this and see if you're happy with it? Its a small PR so I'm happy if either @Aurashk or @PawelPlesniak can look into it and approve it. It has to go in after the listed dependency tho so we can wait until that gets merged before this can go in. |
| "SSH_SHELL_process_manager": [ | ||
| "was terminated unexpectedly through the remote pid by a SIGKILL", | ||
| ], |
There was a problem hiding this comment.
Flagging this up for the review of DUNE-DAQ/integrationtest#158
Aurashk
left a comment
There was a problem hiding this comment.
lgtm, only one thing to mention below
| re.compile(r"Process 'mlt'.*?process exited\s+with exit code 0", re.DOTALL), | ||
| re.compile( | ||
| r"Process 'mlt' \(.*?\) was terminated by the process manager through the remote pid\. Reported exit code: 0\.", re.DOTALL | ||
| ), |
There was a problem hiding this comment.
My only concern is this line, I'm not sure we've consistently got things so that the remote pid is always retrieved successfully for a clean exit. Although this is strictly better as it's more up to date so would support merging this
There was a problem hiding this comment.
Thanks Aurash, thats useful information to know!
My two cents:
- Was unaware that we might not have a consistent thing for the remote pid
- For this integtest I think its fine
- integtest is not a blocker for most PRs to merge
- this (transiently?) failing would be good to keep track of the issue until the remote pid issue gets properly fixed
|
I'll consider this approved and ready to go in now. Since this is a coordinated merge, I'll announce in the relevant slack channels for ppl to know about this. Fingers crossed I'll go and get this merged by tomorrow 10 am UK time or so |
|
Confirmed the expected behaviour, so this can be announced today |
Description
Fixes #932
See above for a description of the problem.
Also added another quick change in which we now have the
--verbosityoption in the drunc integ test. by default, it set to whatever the default is so the output in the terminal is truncated. But for debugging, it is useful to set the verbosity level explicitly. I find that5works well.Type of change
List of required branches from other repositories
Will need to go in with or after this goes in
DUNE-DAQ/integrationtest#158
Suggested manual testing checklist
Check out this branch
Check out the linked PR
Make sure they are both in the pythoncode directory
Run
./scripts/drunc_integtest_bundle.sh. See its all green./scripts/drunc_integtest_bundle.sh --verbosity 5. See that extra messages are being printed out in the terminal.Developer checklist
Prior to marking this as "Ready for Review"
Tests ran on: np04-srv-028 from release NFD_DEV_260526_A9
pytest --marker) passeddaqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.pydaqsystemtest_integtest_bundle.sh./scripts/drunc_integtest_bundle.sh)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