Skip to content

Introduce generalized straight line programs#88

Merged
cdwensley merged 12 commits intogap-packages:masterfrom
ThomasBreuer:TB_gslp
Mar 31, 2026
Merged

Introduce generalized straight line programs#88
cdwensley merged 12 commits intogap-packages:masterfrom
ThomasBreuer:TB_gslp

Conversation

@ThomasBreuer
Copy link
Copy Markdown
Contributor

No description provided.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 16, 2026

Codecov Report

❌ Patch coverage is 99.31507% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.21%. Comparing base (4f3107a) to head (cbcf781).
⚠️ Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
lib/gslp.gi 99.25% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #88      +/-   ##
==========================================
+ Coverage   83.40%   85.21%   +1.81%     
==========================================
  Files          29       32       +3     
  Lines        1693     1867     +174     
==========================================
+ Hits         1412     1591     +179     
+ Misses        281      276       -5     
Files with missing lines Coverage Δ
lib/gslp.gd 100.00% <100.00%> (ø)
makedoc.g 100.00% <100.00%> (ø)
lib/gslp.gi 99.25% <99.25%> (ø)

... and 1 file 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
Copy Markdown
Contributor Author

The missing code coverage is due to the fact that examples from the manual are not tested automatically.

I see three possibilities to improve this behaviour.

  1. Copy these examples to an ordinary testfile. (Easy for the moment, but updating this file by hand will sooner of later become nasty.)
  2. Add automation to create/update such a testfile whenever the manual gets built.
  3. Use GAPDoc's ExtractExamples and RunExamples in the CI tests, in order to run the code from examples in the manual without putting them into a file.

(Using variant 3., I noticed that several manual examples in the package are in fact not up to date.
This must be fixed.
A logical problem is that doc/intro.xml advertises the call ReadPackage( "utils", "tst/testall.g" ). This cannot be a testable example because it terminates the GAP session. Is there a better way to solve this problem than to use <Log> instead of <Example>?)

@fingolfin
Copy link
Copy Markdown
Member

fingolfin commented Mar 16, 2026

We just talked about this on video, but just to write this one remark down: you can also ask AutoDoc to extract the examples (which boils down to invoking the GAPDoc code):

diff --git a/makedoc.g b/makedoc.g
index 21376e7..24f46c1 100644
--- a/makedoc.g
+++ b/makedoc.g
@@ -8,6 +8,7 @@ LoadPackage( "AutoDoc" );
 
 AutoDoc( rec( 
+    extract_examples := true,
     scaffold := rec(
         ## MainPage := false, 
         includes := [ "intro.xml",
                       "print.xml",

(There are also options one can pass to extract_examples but I hope the default will be fine here).

And yeah, ReadPackage( "utils", "tst/testall.g" ) should be in a <Log>

@fingolfin
Copy link
Copy Markdown
Member

(Sorry I pasted the wrong patch first, fixed it now)

- Use `extract_examples` when building the manual
  in order to create testfiles for the manual examples
  (and thus to make these examples relevant for code coverage),
- adjust some manual examples that were not correct,
- turn some not testable manual examples into `<Log>` blocks,
- move `<Example>` blocks into the `<Description>` blocks
  of `<ManSection>`s,
- fix a few typos.
I take this as an argument
that it would be better to use GAPDoc's `ExtractExamples`/`RunExamples`
directly, without the need to create testfiles.
Different GAP versions choose different generating sets for a group.
@cdwensley
Copy link
Copy Markdown
Collaborator

Apologies that I had not noticed that test examples and the manual were not in agreement.
You folks have much better things to do so - if you agree - I am very happy to merge this and fix the test errors.
Please let me know.

The output depends on the set of loaded packages.
Under the conditions of the CI tests, the output chosen now appears.
@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

This is really ugly:

Before my most recent commits, the CI tests called GAP with the -A option (GAP='/home/runner/gap/gap -l /tmp/gaproot; --quitonbreak -A --cover coverage/Q9sc4G.coverage'), and all tests agreed that there are different outputs in three examples.

The most recent CI test run called GAP without the -A option (GAP='/home/runner/gap/gap -l /tmp/gaproot; --quitonbreak --cover coverage/Tof6gT.coverage'), and differences are reported in the same places because I had adjusted the tests to the previous test situation.

The current test examples cannot be used for both situations.

(The different behaviour is due to the fact that the crisp package installs a highly ranked method for NormalSubgroups, the effect here is that the list of normal subgroups is sorted in a different way.)

such that the output does not depend on which packages are loaded
@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

The CI tests for some (but not all) GAP versions fail on Download( "https://www.gap-system.org/index.html" ) from tst/download.tst, and the failed tests take much more time than the successful ones.

(Perhaps this helps to avoid the test failures.)
@cdwensley
Copy link
Copy Markdown
Collaborator

Should I now be merging this, or is that something you can do when you are ready?

@ThomasBreuer
Copy link
Copy Markdown
Contributor Author

Let us wait some time, perhaps somebody wants to comment on the contents.
(Tests were always failing until yesterday evening, so there was no motivation to comment.)

@cdwensley
Copy link
Copy Markdown
Collaborator

I was hoping to look at the new material, and perhaps comment on it, but have not been able to download this branch.

@ThomasBreuer ThomasBreuer requested a review from fingolfin March 25, 2026 13:54
Copy link
Copy Markdown
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

This seems generally fine to me and could be merged, @cdwensley

I am not going to claim I checked every line carefully -- I didn't. But it is (unsurprisingly) looking very well and solid, and even if there are issues hidden in there, I am sure you can count on Thomas addressing them.

@cdwensley
Copy link
Copy Markdown
Collaborator

Will merge and then have a look myself.

@cdwensley cdwensley merged commit 63ccff0 into gap-packages:master Mar 31, 2026
7 checks passed
@ThomasBreuer ThomasBreuer deleted the TB_gslp branch March 31, 2026 13:34
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.

3 participants