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 some tests for permutations #1145

Merged
merged 4 commits into from
Apr 10, 2017

Conversation

fingolfin
Copy link
Member

This PR begins adding additional tests for permutations, working systematically through src/permutat.c.

We might want to add similar tests for partial permutations and transformations, but I know little about them.

@fingolfin fingolfin added the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Feb 15, 2017
@codecov
Copy link

codecov bot commented Feb 15, 2017

Codecov Report

Merging #1145 into master will increase coverage by 0.37%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1145      +/-   ##
==========================================
+ Coverage   59.58%   59.95%   +0.37%     
==========================================
  Files         934      933       -1     
  Lines      292154   290265    -1889     
  Branches    18148    17611     -537     
==========================================
- Hits       174072   174032      -40     
+ Misses     113713   111868    -1845     
+ Partials     4369     4365       -4
Impacted Files Coverage Δ
src/permutat.c 53.31% <100%> (+2.26%) ⬆️
hpcgap/src/funcs.c 48.74% <0%> (-0.53%) ⬇️
hpcgap/src/vars.c 37.84% <0%> (-0.18%) ⬇️
src/scanner.c 70.16% <0%> (+0.08%) ⬆️
src/hpc/threadapi.c 28.21% <0%> (+0.25%) ⬆️
src/hpc/tls.h 67.5% <0%> (+2.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcb0002...279023c. Read the comment docs.

@markuspf
Copy link
Member

fwiw trans.c has coverage of 99+%, its one of the model citizens of src/..

@fingolfin
Copy link
Member Author

Excellent -- I didn't check before writing that remark (I actually only thought of it when writing th PR description, should have checked). Anyway, I actually see trans.c at "only" 95-96% (I am not complaining; there are always some bits that are tough to reach). Also pperm.c has a respectable 85-86 while for permutat.c it's only 44-55%. (I give number ranges, because I see something different on master vs. https://codecov.io/gh/gap-system/gap/tree/7de6e7274e9a91fc80dd5f35ede176be181c32d9/src but I hope this will stabilize once PR #1143 is ready and merged)

@markuspf
Copy link
Member

#1130 takes it to literally 99% though (and I only know this because we had the discussion about test coverage today ;))

@fingolfin fingolfin removed the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Apr 9, 2017
@fingolfin fingolfin requested review from ChrisJefferson and removed request for ChrisJefferson April 9, 2017 22:38
@ChrisJefferson ChrisJefferson merged commit a5e446b into gap-system:master Apr 10, 2017
@fingolfin fingolfin deleted the mh/permtest branch April 10, 2017 19:49
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