Skip to content
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 two tests for trivial cases which cause errors when nilmat is loaded #632

Merged
merged 1 commit into from
Oct 21, 2017

Conversation

bh11
Copy link
Contributor

@bh11 bh11 commented Feb 23, 2016

When nilmat (v. 1.2) is loaded, the two added tests cause Errors. Do not merge before nilmat gets updated. The same error is also triggerred by CRISP automatic tests when nilmat is loaded.

This resolves #631.

@bh11 bh11 added the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Feb 23, 2016
@olexandr-konovalov
Copy link
Member

New release of NilMat 1.3 is published at http://larmor2.nuigalway.ie/~dane/nilmat/ and is now waiting to be picked up and tested. Let's see what will happen.

@olexandr-konovalov
Copy link
Member

Tests pass with Nilmat 1.3. This PR needs rebasing to resolve conflicts and may be closed as soon as Nilmat 1.3 will appear in the merged packages archive for the master branch (switch to this archive planned in #1714).

@olexandr-konovalov
Copy link
Member

@bh11 would you be able to rebase this, please?

@bh11 bh11 removed the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Oct 19, 2017
@codecov
Copy link

codecov bot commented Oct 19, 2017

Codecov Report

Merging #632 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #632      +/-   ##
==========================================
- Coverage   62.82%   62.82%   -0.01%     
==========================================
  Files         969      969              
  Lines      295200   295201       +1     
  Branches    13050    13050              
==========================================
- Hits       185453   185447       -6     
- Misses     106951   106953       +2     
- Partials     2796     2801       +5
Impacted Files Coverage Δ
lib/queue.g 66.4% <0%> (-3.2%) ⬇️
src/hpc/traverse.c 77.9% <0%> (-0.7%) ⬇️
src/funcs.c 71.82% <0%> (-0.44%) ⬇️
src/hpc/thread.c 46.83% <0%> (-0.2%) ⬇️
src/lists.c 56.6% <0%> (-0.12%) ⬇️
src/gap.c 56.83% <0%> (+0.08%) ⬆️
src/stats.c 75.47% <0%> (+0.13%) ⬆️
src/system.c 53.27% <0%> (+0.33%) ⬆️

Copy link
Member

@olexandr-konovalov olexandr-konovalov left a comment

Choose a reason for hiding this comment

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

Thanks @bh11 - looks good now.

Copy link
Member

@olexandr-konovalov olexandr-konovalov left a comment

Choose a reason for hiding this comment

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

The only minor comment is the commit message which has too long first line which then is displayed with a bad line break. What about:

Add two test cases for a trivial matrix group

These tests were broken when NilMat 1.2 was loaded. The problem has been fixed in NilMat 1.3.

Closes #631.

These tests were broken when NilMat 1.2 was loaded. The problem has been fixed in NilMat 1.3.
@olexandr-konovalov
Copy link
Member

@bh11 thanks again!

@olexandr-konovalov olexandr-konovalov merged commit a18628c into gap-system:master Oct 21, 2017
@olexandr-konovalov olexandr-konovalov added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Jan 21, 2018
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nilmat: with package loaded, valid code suddenly runs into an error
2 participants