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

False positive on MOAB/DAGMC CI tests #1538

Closed
gonuke opened this issue May 5, 2024 · 5 comments
Closed

False positive on MOAB/DAGMC CI tests #1538

gonuke opened this issue May 5, 2024 · 5 comments
Assignees

Comments

@gonuke
Copy link
Contributor

gonuke commented May 5, 2024

Describe the Bug

CI tests report success for MOAB & DAGMC stages even if MOAB is not found.

Because PyNE has been written to be compliant, it gracefully continues to build even if MOAB is not present/found. Relevant unit tests are configured to be skipped if MOAB/DAGMC are not found, and thus don't report failure.

We need to establish a test regimen that results in failure if MOAB is not found. Perhaps a build option to make MOAB "REQUIRED"?

See #1537 for user experience.

@ahnaf-tahmid-chowdhury
Copy link
Member

I have already fixed these issues in the scikit-build-core branch. I also added new features to download optional packages if users requested them. I am now working on solving #1536, and after that, I hope all the tests will pass on my branch and ready to make a new PR on the develop branch.

@gonuke
Copy link
Contributor Author

gonuke commented May 17, 2024

We should update our setup/build process to make MOAB and DAGMC required if they are requested at build time - should only require a small change to CMakeLists.txt

@ahnaf-tahmid-chowdhury
Copy link
Member

I have already addressed this in #1526. I also believe we need to update the FindMOAB file as well to support the latest version of MOAB. Please let me know how you would like to proceed.

@gonuke
Copy link
Contributor Author

gonuke commented May 18, 2024

Thanks @ahnaf-tahmid-chowdhury - I am now better understanding all the issues. I'll review #1526 for merge soon

@ahnaf-tahmid-chowdhury
Copy link
Member

Fixed #1526

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants