[bieryAtFnal]

... and updated the existing integtests to react to changes in the integrationtest infrastructure.

Description

These changes are part of the work to add support for multiple user-specified application in our integration tests. The umbrella PR for these changes is DUNE-DAQ/integrationtest#168, and that PR has suggested instructions for testing these changes. All tests of the changes in this repository will need to include the changes in the integrationtest repo.

One of the reasons for creating this PR in "draft" mode is that there are still some changes needed in the process_manager_test.py in order to get it to work.

Type of change

• New feature / enhancement

List of required branches from other repositories

integrationtest kbiery/multi_ctrl_proc_support

Change log

The two existing integtests have been updated, and a new integtest has been added.

Suggested manual testing checklist

Please see DUNE-DAQ/integrationtest#168.

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_bundle requires 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 .

• Unit tests (pytest --marker) passed
  • With relevant marker
  • Without marker
• Integration tests passed
  • Only daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.py
  • Full daqsystemtest_integtest_bundle.sh
• Testing skipped as there are no core code changes in this PR, this only relates to documentation/CI workflows
• Drunc integration tests pass (./scripts/drunc_integtest_bundle.sh)

Final checklist prior to marking this as "Ready for Review"

• Code is clearly commented.
• New unit tests have been added, or is documented in # ISSUE NUMBER
• A suitable reviewer has been chosen from this list.

Reviewer checklist

• This branch has been rebased with develop prior to testing.
• Suggested manual tests show changes.
• CI workflows fails documented (if present)
• Integration tests passed (on either np0x or IC HEP clusters)
  • Use the following guidelines to determine which of the integration tests you need to run
    • You do not need to run any integration tests if
      • Code changes are not associated with src/
      • PR changes only affect docstrings
      • In this case, be sure to validate any suggested manual testing.
    • Run only the minimum integration test as daqsystemtest_integtest_bundle.sh -k minimal_system_quick_test.py if
      • PR changes only affect a few log entries
      • PR changes are small, and do not have a large impact on the workflow (use carefully)
    • Otherwise run the full integration test bundle as daqsystemtest_integtest_bundle.sh
  • What to do if the integration tests fail?
    • Only concern yourself if failures related to drunc are in the log files
    • If non-drunc failure appears:
      • Validate failure in fresh working area
      • Contact Pawel if unsure
• If you have ran the full integration test bundle, leave a comment on the PR stating
  • Which host the integration tests have ran on
  • [Optional] A copy of the test summary
• Drunc integration tests pass (scripts/drunc_integtest_bundle.sh)

Once the above boxes are checked, the PR(s) can be merged following the steps below.

Prior to merging

Choose one of the following an complete all substeps

• Changes only affect the Run Control, are in a single repository, and do not affect the end user.
  • Changes are documented in docstrings and code comments
  • Wiki has been updated if architectural or endpoint changes
• Otherwise
  • Workflow changes demonstrated in the Change Log (if necessary)
  • Wiki has been updated (if necessary)
  • #dunedaq-integration Slack channel notified (see below)

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

The CCM WG has an isolated PR ready to merge that affects user workflows. The PR is:

_URL_

I will leave time for any comments, otherwise will merge these at the end of the work day _Insert your time zone_.

For co-ordinated merge

The CCM WG has a set of co-ordinated merges ready to merge. The PRs are:

_URL_

_URL_


I will leave time for any comments, otherwise will merge these at the end of the day.

[bieryAtFnal]

@emmuhamm , thanks for taking a look.

Regarding the command prompt that is now present at the beginning of the output lines and is tripping up the process_manager_test...

• thanks for identifying that
• as you probably guessed, it is a side effect of the different way that commands are send to the run control apps when the apps are user-specified. The commands are fed into the apps one-at-a-time, compared with being given to the drunc-unified-shell on the command line (batch mode, I think) when users don't specify which apps to run.
• I can look into suppressing the command prompt in the console output that is returned to integtest "tests". Should I do that?
• are there other new characteristics of the console output that is passed back to the integtest "tests" that you've noticed and would like me to change?

Regarding the new integtests that were added to the drunc repo recently, yes, I will update them to use the new completed_processes list.

[emmuhamm]

Hi Kurt, indeed I had a feeling that this was a side effect on how they're sent to the run control apps.

I can look into suppressing the command prompt in the console output that is returned to integtest "tests". Should I do that?

I assume this refers to run_dunerc.completed_processes["drunc"].stdout? If so, my gut says that might be a good idea. Since we call it via completed_process[name] anyway, maybe having the drunc-unified-shell > be removed might be nice. However, if it's harder than expected I wouldn't be too worried about it.

Saying that, I think that the regex in the process manager test should be fixed, which can be done by removing the ^ at the beginning. Can you include this fix in this PR if you don't mind?

are there other new characteristics of the console output that is passed back to the integtest "tests" that you've noticed and would like me to change?

Not that I've noticed. All the tests I've ran seem to run fine, and the PM test that failed I would attribute to a too strict regex anyway. As they are now, they look good to me.

[bieryAtFnal]

I've made both changes that we talked about regarding the initial failure of the process_manager_test...

• I modified the integrationtest infrastructure to strip off the command prompt in the console output
• I removed the caret at the front of the regex in process_manager_test

Independent of that, I have updated the new failure_mode_* integtests in the drunc repo so that they use the completed_processes list of status information.

I'm still running tests, so this is just a status update...

[PawelPlesniak]

My apologies for not looking at this earlier. I am reviewing this PR

[emmuhamm]

Thanks for this Kurt! Most of the code changes look good to me, I only have one comment regarding the src/drunc/integtest/basic_multiapp_test.py file.

Can you ensure that this passes the ruff checks?

Once this is done I can approve this

[bieryAtFnal]

@emmuhamm , I believe that the ruff checks now pass.

[emmuhamm]

Thank you Kurt, this looks good to me! I'll approve this for now, but will hold off on merging until I get through the rest of the correlated PRs

Also as something fun, as can be seen the related stacked PR builds on top of this and works very well :)

[emmuhamm]

LGTM 👍