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.

Sign up for GitHub

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

Jump to bottom

Fix bugs in HPC-GAP serialization code and improve its tests #3761

Merged
merged 2 commits into from
Nov 26, 2019
Merged

Fix bugs in HPC-GAP serialization code and improve its tests #3761

merged 2 commits into from
Nov 26, 2019

Conversation

fingolfin
Copy link
Member

No description provided.

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them topic: HPC-GAP Issues and PRs related to HPC-GAP topic: kernel labels Nov 25, 2019
@rbehrends
Copy link
Contributor

Looks good, minus the problem that test output for object maps/sets depends on ordering?

ChrisJefferson
ChrisJefferson reviewed Nov 26, 2019
View reviewed changes
src/objset.c Outdated Show resolved Hide resolved
tst/testinstall/hpc/serialize.tst Outdated Show resolved Hide resolved
src/hpc/serialize.c Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Nov 26, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.001%) to 84.675% when pulling 06581cf on fingolfin:mh/serialize into b5f7418 on gap-system:master.

@ChrisJefferson
Copy link
Contributor

I'm now happy to merge, but I'll let you squash the fixup! commit first

@fingolfin
kernel: add some type checks for T_OBJSET and T_OBJMAP
e05a7dc
@fingolfin
Fix bugs in HPC-GAP serialization code and improve its tests
06581cf 
In particular, DeserializeObjMap was buggy and created an object *set*.

Serializing a plain record did not take negative rnams into account and
thus would crash or produce garbage.

Both issues were found by improving the tests; specifically, by fixing
them to actually *test* anything -- before, CheckSerialization and
CheckSerialization2 never ever raised an error (unless serialization or
deserialization outright raise an error). Now we check stuff, and this
immediately revealed some bugs (and a few more were revealed by further
tests I added).
@ChrisJefferson ChrisJefferson merged commit 10a05b2 into gap-system:master Nov 26, 2019
@fingolfin fingolfin deleted the mh/serialize branch November 27, 2019 07:47
@PaulaHaehndel PaulaHaehndel self-assigned this Feb 17, 2021
@PaulaHaehndel PaulaHaehndel added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Feb 17, 2021
@PaulaHaehndel PaulaHaehndel removed their assignment Feb 17, 2021
@olexandr-konovalov olexandr-konovalov mentioned this pull request Feb 19, 2021
Closed
@olexandr-konovalov olexandr-konovalov mentioned this pull request Mar 23, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisJefferson ChrisJefferson ChrisJefferson left review comments

@rbehrends rbehrends rbehrends approved these changes

Assignees
No one assigned
Labels
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: HPC-GAP Issues and PRs related to HPC-GAP topic: kernel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@fingolfin @rbehrends @coveralls @ChrisJefferson @PaulaHaehndel