search

svalinn / DAGMC

Direct Accelerated Geometry Monte Carlo Toolkit
https://svalinn.github.io/DAGMC
Other
96 stars 63 forks source link

move test-on-merge with master/develop to be optional #870

Closed gonuke closed 1 year ago

gonuke commented 1 year ago

Description

Change tests against MOAB master and develop to be optional.

Motivation and Context

We current only test on MOAB 5.3.0 during review of a PR, but then test on MOAB master and develop upon merge. Those tests against master & develop are meant to be advisory and not indicative of the state of the project.

Changes

Change github action behavior.

gonuke commented 1 year ago

This may ultimately simplify #863 for @shimwell as well?

gonuke commented 1 year ago

Updated docker image builds available here: https://github.com/gonuke/DAGMC/actions/runs/4439152727

shimwell commented 1 year ago

Looks like the docker builds failed with the python -c "import pymoab" command.

gonuke commented 1 year ago

Yes - I think that's a known failure of MOAB master 🤷

gonuke commented 1 year ago

Actually, that looks like a long standing issue in our docker building... we try to build 5.4.0 in our docker images, but try build the subsequent steps with 5.3.0.

gonuke commented 1 year ago

New docker image build: https://github.com/gonuke/DAGMC/actions/runs/4443513280

gonuke commented 1 year ago

It doesn't make sense to build CI images with MOAB master because (a) it changes regularly and the point is to test against the newest master, and (b) that's why we clone & build MOAB master manually for those cases. I removed that in 667710b

gonuke commented 1 year ago

New docker CI image build: https://github.com/gonuke/DAGMC/actions/runs/4451584603

gonuke commented 1 year ago

Had the wrong python version in the PYTHONPATH - new CI images building here: https://github.com/gonuke/DAGMC/actions/runs/4456756466

gonuke commented 1 year ago

I took out the test of the pymoab install since it isn't needed for DAGMC testing - new image builds here: https://github.com/gonuke/DAGMC/actions/runs/4457395067

shimwell commented 1 year ago

Everything appears to have built and all tests passed

shimwell commented 1 year ago

Is it worth testing with Moab 5.4.0 in the matrix. I think it was taken out during this PR but it might be more important that 5.3.0

gonuke commented 1 year ago

I can try 5.4.0, but I wouldn’t want it to hold this up.

shimwell commented 1 year ago

Ok let us not try 5.4.0 then. I think this PR is good to merge. Shall we wait a little in case anyone else has comments

shimwell commented 1 year ago

thanks for the improvements paul