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

Added pigz to unit tests #985

Merged
merged 3 commits into from
Jun 8, 2021
Merged

Conversation

nmoinvaz
Copy link
Member

@nmoinvaz nmoinvaz commented Jun 4, 2021

This PR adds:

  • New GitHub Actions CI used to test pigz.
  • New CMakeLists.txt that can be used to build pigz against zlib-ng or any other zlib variant.

@codecov
Copy link

codecov bot commented Jun 4, 2021

Codecov Report

Merging #985 (541b2ab) into develop (2c65dce) will increase coverage by 0.64%.
The diff coverage is n/a.

❗ Current head 541b2ab differs from pull request most recent head e16dd54. Consider uploading reports for the commit e16dd54 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #985      +/-   ##
===========================================
+ Coverage    75.81%   76.45%   +0.64%     
===========================================
  Files           74       74              
  Lines         8082     8317     +235     
  Branches      1343     1369      +26     
===========================================
+ Hits          6127     6359     +232     
- Misses        1425     1427       +2     
- Partials       530      531       +1     
Flag Coverage Δ
macos_clang 68.54% <ø> (-0.01%) ⬇️
macos_gcc 67.47% <ø> (+0.01%) ⬆️
ubuntu_clang 69.48% <ø> (?)
ubuntu_clang_debug 68.89% <ø> (∅)
ubuntu_clang_inflate_allow_invalid_dist 69.23% <ø> (?)
ubuntu_clang_inflate_strict 69.48% <ø> (?)
ubuntu_clang_mmap 69.20% <ø> (?)
ubuntu_clang_msan 69.48% <ø> (∅)
ubuntu_clang_pigz_no_optim 37.64% <ø> (?)
ubuntu_gcc 68.72% <ø> (+0.01%) ⬆️
ubuntu_gcc_aarch64 69.00% <ø> (+0.01%) ⬆️
ubuntu_gcc_aarch64_compat_no_opt 67.31% <ø> (-0.02%) ⬇️
ubuntu_gcc_aarch64_no_acle 67.91% <ø> (+0.02%) ⬆️
ubuntu_gcc_aarch64_no_neon 68.31% <ø> (+0.02%) ⬆️
ubuntu_gcc_armhf 68.99% <ø> (+0.01%) ⬆️
ubuntu_gcc_armhf_compat_no_opt 67.30% <ø> (-0.02%) ⬇️
ubuntu_gcc_armhf_no_acle 69.15% <ø> (+0.01%) ⬆️
ubuntu_gcc_armhf_no_neon 69.48% <ø> (+0.02%) ⬆️
ubuntu_gcc_armsf 69.00% <ø> (+0.01%) ⬆️
ubuntu_gcc_armsf_compat_no_opt 67.30% <ø> (-0.02%) ⬇️
ubuntu_gcc_compat_no_opt 68.69% <ø> (-0.02%) ⬇️
ubuntu_gcc_mingw_i686 0.00% <ø> (ø)
ubuntu_gcc_mingw_x86_64 0.00% <ø> (ø)
ubuntu_gcc_no_avx2 69.07% <ø> (+0.01%) ⬆️
ubuntu_gcc_no_pclmulqdq 66.90% <ø> (+0.01%) ⬆️
ubuntu_gcc_no_sse2 68.13% <ø> (+0.01%) ⬆️
ubuntu_gcc_no_sse4 66.86% <ø> (+0.01%) ⬆️
ubuntu_gcc_o3 68.45% <ø> (+0.01%) ⬆️
ubuntu_gcc_osb 70.99% <ø> (+0.01%) ⬆️
ubuntu_gcc_ppc 69.18% <ø> (+0.02%) ⬆️
ubuntu_gcc_ppc64 69.97% <ø> (+0.02%) ⬆️
ubuntu_gcc_ppc64le 68.67% <ø> (+0.02%) ⬆️
ubuntu_gcc_s390x 67.92% <ø> (+0.01%) ⬆️
ubuntu_gcc_sparc64 70.37% <ø> (+0.02%) ⬆️
win64_gcc 70.40% <ø> (+0.01%) ⬆️
win64_gcc_compat_no_opt 73.87% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
inffast.c 88.27% <0.00%> (-6.87%) ⬇️
test/adler32_test.c 63.63% <0.00%> (-6.37%) ⬇️
zutil.c 35.71% <0.00%> (-1.79%) ⬇️
uncompr.c 79.06% <0.00%> (-1.42%) ⬇️
test/minideflate.c 59.44% <0.00%> (-1.01%) ⬇️
inflate.c 87.41% <0.00%> (-0.78%) ⬇️
crc32_comb.c 82.22% <0.00%> (-0.71%) ⬇️
test/switchlevels.c 59.00% <0.00%> (-0.60%) ⬇️
deflate.c 71.22% <0.00%> (-0.14%) ⬇️
test/infcover.c 90.69% <0.00%> (-0.09%) ⬇️
... and 30 more

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 2c65dce...e16dd54. Read the comment docs.

@nmoinvaz
Copy link
Member Author

nmoinvaz commented Jun 4, 2021

If we add the files in https://github.com/neurolabusc/zlib-bench/tree/master/corpus to https://github.com/zlib-ng/corpora repository then we can also provide test coverage for old pigz issue #536. @Dead2 what do you think? Those files are 18mb and 34mb.

@mtl1979
Copy link
Collaborator

mtl1979 commented Jun 4, 2021

I'm all for adding tests that make sure we don't introduce regressions or tests that confirm we have an crash issue.

@nmoinvaz
Copy link
Member Author

nmoinvaz commented Jun 4, 2021

I might just create a new repository corpora-lg for larger test corpora.

@mtl1979
Copy link
Collaborator

mtl1979 commented Jun 4, 2021

@nmoinvaz I usually just clone the original repository and then rename it... That's a lot easier than trying to combine files from multiple repositories and preserving edit history to conform to copyrights.

@nmoinvaz
Copy link
Member Author

nmoinvaz commented Jun 4, 2021

I have added a CI test for no threads to be able isolate threaded compression and also a CI test for no optimizations to isolate those as well.

@nmoinvaz
Copy link
Member Author

nmoinvaz commented Jun 4, 2021

@mtl1979 it could be as simple as a GitHub CI action that will download the new repo (same as what is done with corpora). CMake script picks up everything in test/data/**.

@mtl1979
Copy link
Collaborator

mtl1979 commented Jun 4, 2021

I'm not big fan of supporting pigz but if it helps finding issues involving corner cases, then I see no reason to "oppose"...

Keeping own "version" of repositories is always a good thing as the original repository might go offline and break our build system...

@nmoinvaz
Copy link
Member Author

nmoinvaz commented Jun 4, 2021

If we ended up include the pigz sources in our repository then we will likely have to support it, that is why I pull the official sources from madler/pigz. We could fork that repo, but I don't want to have to keep it up to date.

@nmoinvaz
Copy link
Member Author

nmoinvaz commented Jun 4, 2021

Regarding neuro images, I have decided to just added them to corpora repository (also I don't want to manage yet another repo). Even though it is an extra 80mb it will be good to have it tested every where because image data such as that is unique and good for RLE tests. So now this PR should provide coverage for #536.

@Dead2
Copy link
Member

Dead2 commented Jun 8, 2021

Looks good to me. Is this ready to merge?

@nmoinvaz
Copy link
Member Author

nmoinvaz commented Jun 8, 2021

Yes it is. The CI tests that fail, fail because our code fails them.

@Dead2 Dead2 merged commit c58d995 into zlib-ng:develop Jun 8, 2021
Dead2 added a commit that referenced this pull request Jun 11, 2021
- Minor code cleanup #983 #984
- Fix mpicc compilation #959
- Fix build on NetBSD #964
- Fix build on OpenBSD #970
- Fix build on Cygwin #972 #974
- Fix linter warnings in configure #975
- Spelling fixes #961
- Improve unistd.h handling #960
- Remove stdarg.h detection #976
- CI/Test improvements #977 #981 #985
- Cmake improvements #980
Dead2 added a commit that referenced this pull request Jun 11, 2021
- Minor code cleanup #983 #984
- Fix mpicc compilation #959
- Fix build on NetBSD #964
- Fix build on OpenBSD #970
- Fix build on Cygwin #972 #974
- Fix linter warnings in configure #975
- Spelling fixes #961
- Improve unistd.h handling #960
- Remove stdarg.h detection #976
- CI/Test improvements #977 #981 #985
- Cmake improvements #980 #989
Dead2 added a commit that referenced this pull request Jun 11, 2021
- Fix inflate corruption #982
- Minor code cleanup #983 #984
- Fix mpicc compilation #959
- Fix build on NetBSD #964
- Fix build on OpenBSD #970
- Fix build on Cygwin #972 #974
- Fix linter warnings in configure #975
- Spelling fixes #961
- Improve unistd.h handling #960
- Remove stdarg.h detection #976
- CI/Test improvements #977 #981 #985
- Cmake improvements #980 #989
@Dead2 Dead2 mentioned this pull request Jun 11, 2021
Dead2 added a commit that referenced this pull request Jun 11, 2021
- Fix inflate corruption #982
- Minor code cleanup #983 #984
- Fix mpicc compilation #959
- Fix build on NetBSD #964
- Fix build on OpenBSD #970
- Fix build on Cygwin #972 #974
- Fix linter warnings in configure #975
- Spelling fixes #961
- Improve unistd.h handling #960
- Remove stdarg.h detection #976
- CI/Test improvements #977 #981 #985
- Cmake improvements #980 #989
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants