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

Fix crash when tracing buggy attributes/properties; add a bunch of tests #2711

Merged
merged 3 commits into from
Aug 19, 2018

Conversation

fingolfin
Copy link
Member

No description provided.

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) topic: tests issues or PRs related to tests topic: kernel release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 17, 2018
@codecov
Copy link

codecov bot commented Aug 17, 2018

Codecov Report

Merging #2711 into master will increase coverage by 0.09%.
The diff coverage is 60%.

@@            Coverage Diff             @@
##           master    #2711      +/-   ##
==========================================
+ Coverage   75.66%   75.75%   +0.09%     
==========================================
  Files         478      478              
  Lines      241441   241443       +2     
==========================================
+ Hits       182678   182908     +230     
+ Misses      58763    58535     -228
Impacted Files Coverage Δ
src/macfloat.c 98.76% <ø> (+17.66%) ⬆️
src/opers.c 95.15% <60%> (+0.89%) ⬆️
src/stats.c 89.62% <0%> (+0.19%) ⬆️
src/c_type1.c 84.04% <0%> (+0.63%) ⬆️
hpcgap/lib/hpc/stdtasks.g 64.96% <0%> (+1.53%) ⬆️
src/hpc/c_type1.c 84.66% <0%> (+1.9%) ⬆️
src/records.c 84.54% <0%> (+16.42%) ⬆️
src/ariths.c 92.92% <0%> (+18.72%) ⬆️
... and 1 more

@fingolfin fingolfin requested a review from stevelinton August 18, 2018 12:24
If an attribute or property returns nothing (resp. something other than true
or false), this is an error, which we normally catch. But when tracing is
enabled for the relevant operations, we would instead crash.
Copy link
Contributor

@ChrisJefferson ChrisJefferson left a comment

Choose a reason for hiding this comment

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

That is some extensive testing :)

@fingolfin fingolfin merged commit c8104cd into gap-system:master Aug 19, 2018
@fingolfin fingolfin deleted the mh/tests branch August 19, 2018 21:54
@fingolfin fingolfin added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel topic: tests issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants