illinois-ceesd / mirgecom

MIRGE-Com is the workhorse simulation application for the Center for Exascale-Enabled Scramjet Design at the University of Illinois.
Other
11 stars 19 forks source link

Sanity check CI #939

Closed majosm closed 1 year ago

majosm commented 1 year ago

Seeing some strange things in #924...

majosm commented 1 year ago

@illinois-ceesd/developers Anyone have any guesses as to how the CI here could be picking up tests from main that don't yet exist in this branch? I saw this in #924 and was perplexed. CI finds 897 tests; when I run pytest on this branch locally I see 892. It looks like one of them is a doc test that I'm not running locally, but I noticed in CI it's also running the radiation tests:

2.92s call     test/test_multiphysics.py::test_thermally_coupled_fluid_wall_with_radiation[<PyOpenCLArrayContext for <pyopencl.Device 'cpu-Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz' on 'Portable Computing Language'>>-True-3]

...which don't yet exist in this branch. 🤯 Anyone have any idea what's going on here? The fix is easy enough (just update the branch), but I'm pretty confused by this behavior.

matthiasdiener commented 1 year ago

And for some reason, now the CI doesn't run at all?! 🤯

majosm commented 1 year ago

@matthiasdiener I moved the debug insertions around a little to avoid the conflict. Looks like CI is running now.

matthiasdiener commented 1 year ago

From the most recent log:

2023-07-28T15:15:09.9868687Z ##[group]Determining the checkout info
2023-07-28T15:15:09.9869286Z ##[endgroup]
2023-07-28T15:15:09.9869809Z ##[group]Checking out the ref
2023-07-28T15:15:09.9870740Z [command]/usr/bin/git checkout --progress --force refs/remotes/pull/939/merge
2023-07-28T15:15:10.0110270Z Note: switching to 'refs/remotes/pull/939/merge'.
2023-07-28T15:15:10.0110597Z 
2023-07-28T15:15:10.0111022Z You are in 'detached HEAD' state. You can look around, make experimental
2023-07-28T15:15:10.0111665Z changes and commit them, and you can discard any commits you make in this
2023-07-28T15:15:10.0112309Z state without impacting any branches by switching back to a branch.
2023-07-28T15:15:10.0112653Z 
2023-07-28T15:15:10.0112922Z If you want to create a new branch to retain commits you create, you may
2023-07-28T15:15:10.0113636Z do so (now or later) by using -c with the switch command. Example:
2023-07-28T15:15:10.0113961Z 
2023-07-28T15:15:10.0114219Z   git switch -c <new-branch-name>
2023-07-28T15:15:10.0114483Z 
2023-07-28T15:15:10.0114649Z Or undo this operation with:
2023-07-28T15:15:10.0114895Z 
2023-07-28T15:15:10.0115011Z   git switch -
2023-07-28T15:15:10.0115218Z 
2023-07-28T15:15:10.0115535Z Turn off this advice by setting config variable advice.detachedHead to false
2023-07-28T15:15:10.0115913Z 
2023-07-28T15:15:10.0116300Z HEAD is now at 826792e Merge 0f3dee328c011b774bda968d97d4f8b30cfc81c1 into 8e0468181cd67ea4a5059840f33d85ebd2aeea53
2023-07-28T15:15:10.0125377Z ##[endgroup]

So this merges 0f3dee328c011b774bda968d97d4f8b30cfc81c1 (ie, current state of ci-sanity-check) into 8e0468181cd67ea4a5059840f33d85ebd2aeea53 (ie, current state of main).

majosm commented 1 year ago

So this merges 0f3dee3 (ie, current state of ci-sanity-check) into 8e04681 (ie, current state of main).

Do we know if that's Github doing that or the install script?

matthiasdiener commented 1 year ago

So this merges 0f3dee3 (ie, current state of ci-sanity-check) into 8e04681 (ie, current state of main).

Do we know if that's Github doing that or the install script?

This is done by the actions/checkout step (ie, not our install script), e.g. here. Is this the behavior we expect? This PR is targeting the main branch of mirgecom, so the CI tests merge this branch into the current main, which would be the same behavior if this PR was actually merged, I think.

majosm commented 1 year ago

So this merges 0f3dee3 (ie, current state of ci-sanity-check) into 8e04681 (ie, current state of main).

Do we know if that's Github doing that or the install script?

This is done by the actions/checkout step (ie, not our install script), e.g. here. Is this the behavior we expect? This PR is targeting the main branch of mirgecom, so the CI tests merge this branch into the current main, which would be the same behavior if this PR was actually merged, I think.

If Github does it, then I guess it's fine. 🤷‍♂️ Just surprising to see different tests being run when running locally vs. here. And Github saying it's out of date but then it's magically up to date when CI runs is kind of jarring. 🙂 Oh well.