publish workflow fails but build/test workflow succeeds since #610

[gonuke]

The build/test workflow was successful when #610 was reviewed and then once it was merged. However, the publish workflow runs on merge and it is failing one test: storage.NormalActiveDormantBuyingSize

[bennibbelink]

Looking into this. The build/test workflow passed in the PR but the tests failed on the push of the merge commit (https://github.com/cyclus/cycamore/actions/runs/9304350646). It shows up as a green check since the workflow steps "soft" fail

[bennibbelink]

I see cyclus/cyclus#1749 was merged just before #610 (between the passing PR build/test workflow and the first failing build/test workflow). @nuclearkatie committed StorageTest.NormalActiveDormantBuyingSize ~4 months ago, did any changes in 1749 affect the behavior of the Storage archetype (and associated tests)?

[nuclearkatie]

Hmm, I don't think it should have

[bennibbelink]

Tests pass locally if I checkout the cyclus commit prior to cyclus/cyclus#1749 being merged.

The failing test expects Quantity values for rows 0, 1, 2 to be 0.61256, 0.62217, 0.39705, respectively. These are the Quantity values produced by the test for the first 10 rows:

[nuclearkatie]

Ah... I think I might know what's going on. Hopefully it'll be fixed with cyclus#1750. cyclus#1749 added the packaging of a resource as it gets traded, which can result in the pushing of a zero quantity resource (the leftover unpackaged material, in the case where there is no leftover material) back onto the resource buffer. This results in the resource table having unexpected zero resources.

I'll keep an eye on this and make sure it does get resolved, with cyclus#1750 or a follow-on PR (cyclus#1761) if there are larger changes to make.

Thanks for pulling that list @bennibbelink, that helped me think about where unexpected zero-size resources might be coming from :)

[bennibbelink]

Thanks @nuclearkatie!! Glad you've got a handle on this, I was pretty stumped 😁

[bennibbelink]

We didn't get to see downstream testing for #1761 due to #1762 causing the conda cyclus builds to fail. #1763 attempts to resolve #1762, but downstream cycamore tests are failing at StorageTest.PackageSplitFirst on all 4 builds. @nuclearkatie do the cycamore tests related to packaging need to be updated to reflect changes in cyclus from #1761?

[nuclearkatie]

I'll take a look

[nuclearkatie]

Ah yes, this is the issue that the order of packaged materials in resources is not fixed, so tests need to be written accordingly. PR #612 addresses this for storage tests on packaging, and cyclus PR cyclus#1750 updates the tests on packaging at the material sell policy level