Run hap and hapcryst's test suites in CI - #32
Open
fingolfin wants to merge 4 commits into
Open
Conversation
Both need polymaking and neither pins an upper bound, so what we do here reaches their released versions unchanged. Their suites are the only thing that tells us whether that still works, and running them by hand is what turned up that master, as it stands, breaks both: hapcryst fails outright and hap fails 26 of its tests, in each case on a global variable polymaking no longer sets. Install the package, check that it really did load this checkout rather than the polymaking shipped with GAP, and run whatever it declares as its TestFile. That check matters: without it the job would pass while testing nothing. hapcryst takes about 20 seconds and hap about a minute, so both are cheap enough to run on every pull request; there is also a nightly run, to catch breakage coming from their side. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
install-pkg installs what it is given and does not resolve dependencies, so both failed to load and the job never reached their tests. Name the whole closure, and print the package loading log if a load still fails, so the next such failure explains itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #32 +/- ##
=======================================
Coverage 72.23% 72.23%
=======================================
Files 15 15
Lines 868 868
=======================================
Hits 627 627
Misses 241 241 🚀 New features to boost your workflow:
|
nq ships a program that has to be compiled, and hap will not load without it; install-pkg only unpacks. build-pkg takes an extra-pkgs list for packages that do not appear in our own PackageInfo.g. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
setup-gap stages the checkout under the GAP root rather than leaving it in GITHUB_WORKSPACE, so requiring the two paths to be equal rejected the very thing it was meant to accept. Compare PackageInfo.g instead, which works whether the checkout is copied or linked. The guard was worth having: it fired rather than letting both jobs pass while testing the polymaking that ships with GAP. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
hap and hapcryst both use polymaking. To make sure we don't break them with our changes, let's run their test suites as part of our CI tests.
Generated with Claude Code