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

Regenerate ROOT data for celeritas tests if missing #1517

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

sethrj
Copy link
Member

@sethrj sethrj commented Nov 20, 2024

I propose this as a simpler alternative to #1516. As the reviewers previously described on slack, running

$ ninja clean
$ ninja
$ ctest

fails because the ROOT files are not regenerated.

With this, ROOT data is regenerated automatically as part of celeritas/test/all (or specifically, the testcel_celeritas library) if are absent when building. Importantly, running ninja doesn't renew the files if they already exist.

@sethrj sethrj added core Software engineering infrastructure minor Minor internal changes or fixes labels Nov 20, 2024
@stognini
Copy link
Member

Tried a fully clean build here. The files existed, but were overridden during the fresh build, leading to

❯ git st
 M ../test/celeritas/data/four-steel-slabs.root
 M ../test/celeritas/data/lar-sphere.root
 M ../test/celeritas/data/simple-cms.root

Copy link

Test summary

 3 844 files   5 936 suites   4m 8s ⏱️
 1 597 tests  1 564 ✅ 28 💤 5 ❌
19 790 runs  19 708 ✅ 75 💤 7 ❌

For more details on these failures, see this check.

Results for commit c84f224.

@sethrj
Copy link
Member Author

sethrj commented Nov 20, 2024

Ooh that's not what we want either...

@sethrj
Copy link
Member Author

sethrj commented Nov 23, 2024

This appears to be happening (and on the CI) because of the many dependencies on the "custom command". Rebuilding the celer-export-geant binary forces the file to become out of date, and cloning/updating some of the other files does the same. This is technically correct behavior, but in practice we don't want to regenerate the files except rarely. I'm not sure what the best thing to do is; maybe #1516 really is the way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Software engineering infrastructure minor Minor internal changes or fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants