Skip to content

add some package tests#1334

Open
ThomasBreuer wants to merge 5 commits intooscar-system:masterfrom
ThomasBreuer:TB_package_tests
Open

add some package tests#1334
ThomasBreuer wants to merge 5 commits intooscar-system:masterfrom
ThomasBreuer:TB_package_tests

Conversation

@ThomasBreuer
Copy link
Member

addresses #1330

The nontrivial bit is to call GAP.Packages.build in a way that the function does something.
The package in question must claim to be not available, thus one cannot simple take a loaded package or a package that has been installed with GAP.Packages.install. The idea is to modify the information in the info records temporarily such that GAP thinks the package is not available.
(This must be done package by package, not once before a loop, because PackageManager calls InitializePackagesInfoRecords, which overwrites the changes in the info records.)

@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.53%. Comparing base (060b40c) to head (21209ad).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1334      +/-   ##
==========================================
+ Coverage   78.02%   79.53%   +1.50%     
==========================================
  Files          61       61              
  Lines        4943     4945       +2     
==========================================
+ Hits         3857     3933      +76     
+ Misses       1086     1012      -74     

see 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ThomasBreuer
Copy link
Member Author

My interpretation of the test failure for macOS is that the orb package is not available there.
I should choose other packages for the test of build_recursive.

@fingolfin
Copy link
Member

Anything for which we provide binaries, we provide for both macOS and Linux equally. Hence if orb is not available on macOS then that's a bug we need to investigate. However, I just tried it on my MacBook and it loads just fine.

So whatever is wrong in CI, it's not because orb is not available in general.

@test ! GAP.Packages.load(path)

# Run package tests.
@test ! GAP.Packages.test("no_such_package")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'd expect this to throw an exception (the input arguments were invalid). The values true / false should be reserved for "package was loaded, teste were run and passed / failed"

Comment on lines 71 to 80
# Make sure that GAP stores package information.
@test GAP.Packages.load(pkg[:name])
# Manipulate GAP's global information such that
# `GAP.Globals.TestPackageAvailability` believes
# the package is not yet loaded.
# (Otherwise `GAP.Packages.build` would do nothing.)
pkg[:pkgloaded] = getproperty(GAP.Globals.GAPInfo.PackagesLoaded, pkg[:name])
GAP.Wrappers.UNB_REC(GAP.Globals.GAPInfo.PackagesLoaded, GAP.RNamObj(String(pkg[:name])))
pkg[:pkginfo] = collect(GAP.Globals.PackageInfo(GapObj(pkg[:name])))
pkg[:avail_test] = [x.AvailabilityTest for x in pkg[:pkginfo]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we first load the package and then go through great lengths to convince GAP that it was not actually loaded in the first place. Why do we do that? The explanation given is

Make sure that GAP stores package information

But what does that mean? As far as I can tell, GAP.Globals.GAPInfo.PackagesInfo.orb is also populated with out that.

@fingolfin
Copy link
Member

Ah -- not all macOS test fails. Only the "CI with GAP" tests. Those test GAP.jl against other GAP versions (namely GAP master and the stable-4.15 branch). And there indeed, orb has not been compiled.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants