Skip to content

Make testinstall work with gap --bare - #6354

Merged
fingolfin merged 1 commit into
masterfrom
mh/test-bare
May 27, 2026
Merged

Make testinstall work with gap --bare#6354
fingolfin merged 1 commit into
masterfrom
mh/test-bare

Conversation

@fingolfin

@fingolfin fingolfin commented Apr 29, 2026

Copy link
Copy Markdown
Member

Resolves #6346

@fingolfin fingolfin added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Apr 29, 2026
@codecov

codecov Bot commented Apr 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.86%. Comparing base (59ae437) to head (69a7a96).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6354      +/-   ##
==========================================
+ Coverage   78.83%   78.86%   +0.02%     
==========================================
  Files         685      685              
  Lines      293554   293553       -1     
  Branches     8648     8648              
==========================================
+ Hits       231432   231519      +87     
+ Misses      60314    60226      -88     
  Partials     1808     1808              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fingolfin
fingolfin force-pushed the mh/test-bare branch 2 times, most recently from a3ea9e6 to 2a6237a Compare May 11, 2026 18:56
@fingolfin
fingolfin force-pushed the mh/test-bare branch 3 times, most recently from 35260cd to 1dc65c5 Compare May 17, 2026 22:24
@fingolfin fingolfin added the topic: ci Anything related to GitHub Actions, Codecov, AppVeyor, Coveralls, Travis, ... label May 17, 2026
@fingolfin
fingolfin marked this pull request as ready for review May 17, 2026 22:27
@fingolfin
fingolfin requested a review from ThomasBreuer May 17, 2026 22:28
@ThomasBreuer

ThomasBreuer commented May 18, 2026

Copy link
Copy Markdown
Contributor

The failure is due to the fact that Test sets the global function DefaultReportDiffColors only if GAPInfo.PackagesLoaded.gapdoc is bound, which is not the case in the "bare" situation.

As far as I see, this is just about a few text constants. We could set them as in the current GAPDoc's TextAttr if GAPDoc is not available.

@fingolfin

Copy link
Copy Markdown
Member Author

I've added a commit to deal with the DefaultReportDiffColors issue. Let's see if it works.

@fingolfin

Copy link
Copy Markdown
Member Author

Only failure now is in tst/testinstall/package.tst which loads mockpkg which in turn "needs" GAPDoc, which then gets loaded and prints a banner.

I won't get that done for GAP 4.16.0. But I've restricted the other remaining changes from this PR into PR #6406

@ThomasBreuer ThomasBreuer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Only failure now is in tst/testinstall/package.tst which loads mockpkg which in turn "needs" GAPDoc, which then gets loaded and prints a banner.

I see: Some tests of package extensions are based on the assumption that some GAPDoc version is already loaded. Simply removing GAPDoc from the list of needed packages of mockpkg is not a solution.

Besides this problem, everything looks well. Thanks for this work.

@fingolfin
fingolfin force-pushed the mh/test-bare branch 2 times, most recently from 5ca1bd4 to 96218d0 Compare May 24, 2026 09:30
@fingolfin fingolfin added this to the GAP 4.17.0 milestone May 26, 2026
@fingolfin
fingolfin force-pushed the mh/test-bare branch 2 times, most recently from 6c18012 to 61cdcac Compare May 26, 2026 12:44
@fingolfin
fingolfin merged commit e404ddd into master May 27, 2026
31 of 33 checks passed
@fingolfin
fingolfin deleted the mh/test-bare branch May 27, 2026 06:07
cdwensley pushed a commit that referenced this pull request Jun 18, 2026
cdwensley pushed a commit that referenced this pull request Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: ci Anything related to GitHub Actions, Codecov, AppVeyor, Coveralls, Travis, ...

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: add a test run with gap --bare

2 participants