Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Derived test module reloads and cleanup #14

Merged
merged 12 commits into from
Nov 25, 2024

Conversation

pdmullen
Copy link
Collaborator

@pdmullen pdmullen commented Nov 21, 2024

Background

Moderate cleanup of CI and fixes regression execution on Macs.

Description of Changes

  • Reload "base" test module in derived tests via importlib (such that we will not worry about future derived tests having shared states dependent on order of execution).
  • Fix Mac regression execution, requires special care of positioning of librebound.so.
  • Remove duplicated code in tests.
  • Fix a few edge cases in CI workflows (hopefully without breaking others...)

Checklist

  • New features are documented
  • Tests added for bug fixes and new features
  • (@lanl.gov employees) Update copyright on changed files

@pdmullen pdmullen changed the title WIP: Derived test module reloads and cleanup Derived test module reloads and cleanup Nov 21, 2024
Copy link
Collaborator

@adamdempsey90 adamdempsey90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe some minor changes

@pdmullen pdmullen changed the title Derived test module reloads and cleanup WIP: Derived test module reloads and cleanup Nov 21, 2024
@pdmullen pdmullen changed the title WIP: Derived test module reloads and cleanup Derived test module reloads and cleanup Nov 23, 2024
Copy link
Collaborator

@brryan brryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the --cwd option to guard against surprises with searching for local executables which I would like to see added, LGTM, approving now, thanks for all this typing @pdmullen, sorry I broke your workflow with --oversubscribe before.

@pdmullen
Copy link
Collaborator Author

Apart from the --cwd option to guard against surprises with searching for local executables which I would like to see added, LGTM, approving now, thanks for all this typing @pdmullen, sorry I broke your workflow with --oversubscribe before.

Addressed in 5e90a73

@brryan brryan merged commit 80cadf7 into lanl:develop Nov 25, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants