-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add GAP distro tests #1067
Add GAP distro tests #1067
Conversation
7088374
to
12ce2c4
Compare
name: Test GAP package distro | ||
|
||
on: | ||
pull_request: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pull_request: |
IMO this shouldn't run on every commit in every PR, as it produces >150 jobs. But I have kept it here for now such that in this PR, we can already see it running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are >150 jobs but apparently they take less than 15 minutes to run? So perhaps we keep them on for now and see what happens?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine with me
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1067 +/- ##
==========================================
- Coverage 75.73% 75.28% -0.46%
==========================================
Files 55 55
Lines 4641 4669 +28
==========================================
Hits 3515 3515
- Misses 1126 1154 +28
|
22935a3
to
7846672
Compare
19c0f2b
to
c5a9136
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Thank you for this @lgoettgens. Could you please rebase it? I'll try to have a closer in the coming week(s) |
c5a9136
to
6ddb758
Compare
Co-authored-by: Max Horn <max@quendi.de>
The |
Hmm, there also was a segfault in the And another in
In order to move things along, how about we disable those for now and merge this, then file issues about these failures and then can investigate these later. |
After re-running it is only |
All distro tests are green now. The one failure is #1111. |
- gap-package: 'help' # test failure in HeLP-4.0/tst/yes_4ti2.tst:39 | ||
- gap-package: 'io' # segfaults, most likely due to child process handling | ||
- gap-package: 'itc' # dependency `xgap` has no jll | ||
- gap-package: 'localizeringforhomalg' # `Error, found no GAP executable in PATH` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also issue #1105 (@ThomasBreuer that's another case where a package needs/wants a GAP executable)
Resolves #1065.
This is dependent on #1066 and thus includes it.I've used the ugly hack from oscar-system/GAP_pkg#15 in
GAP.Packages.test
to avoid GAP packages exiting the process after running their tests. This function could be moved to a package extension onTest
once the minimum required julia version is bumped to 1.8.