-
Notifications
You must be signed in to change notification settings - Fork 35
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
Workaround: Fix test failures on Ubuntu jammy s390x. #63
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! The changes look fine, though I would like one additional change to reduce the size of the diff (see inline comment).
test/zlib/test_zlib.rb
Outdated
@@ -12,7 +12,18 @@ | |||
end | |||
|
|||
if defined? Zlib | |||
class TestZlibDeflate < Test::Unit::TestCase | |||
class Zlib::TestCase < Test::Unit::TestCase | |||
def child_env |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The child_env
passed to subprocesses is not modified, correct? If so, I recommend this be a frozen constant and not a method. You could define it as Zlib::CHILD_ENV
, and skip creating a subclass and modifying the tests to use the subclass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the child_env
is not modified. Sure. Let me modify the PR on your way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rebased fixing it on your way. Now what do you think?
ec99a02
to
532ce51
Compare
Note below is the Travis CI Ubuntu jammy s390x result before this PR on my forked repository. It failed as expected. And below is the Travis CI Ubuntu jammy s390x result after this PR. It passed as expected. |
@hsbt or @nurse It seems Travis CI for ruby/zlib doesn't start unlike Travis CI for ruby/ruby. Could you enable it? Thanks. |
This commit fixes the test failures on the zlib in Ubuntu jammy s390x. According to the <https://packages.ubuntu.com/jammy-updates/zlib1g> - `zlib_1.2.11.dfsg-2ubuntu9.2.debian.tar.xz`, the zlib deb package is applying the patch 410.patch (madler/zlib#410), and configured by `./configure --dfltcc` on Ubuntu jammy s390x. The `--dfltcc` is to enable the deflate algorithm in hardware. It produces a different (but still valid) compressed byte stream, and causes the test failures in ruby/zlib. As a workaround, set the environment variable `DFLTCC=0` disabling the implementation in zlib on s390x to the failing tests. Note we need to test in a child Ruby process with `assert_separately` to test on the `DFLTCC=0` set by the parent Ruby process.
The only choice to test s390x on pull-request is currently Travis CI as far as I know.
532ce51
to
cc88b54
Compare
I rebased the PR using the |
Thanks for your review. Let me merge this PR, though the Travis CI is not enabled for this repository yet. |
Nice work figuring out the root cause. |
Thanks. I was definitely not able to fix the issue by the workaround without @iii-i's help and info at #60 (comment). |
Note I opened another PR #65 that is rework for this PR. |
This PR fixes #60, the test failures on the zlib in Ubuntu jammy s390x.
This PR has 2 commits.
The 1st commit
According to the https://packages.ubuntu.com/jammy-updates/zlib1g -
zlib_1.2.11.dfsg-2ubuntu9.2.debian.tar.xz
, the zlib deb package is applying the patch 410.patch (madler/zlib#410), and configured by./configure --dfltcc
on Ubuntu jammy s390x. The--dfltcc
is to enable the deflate algorithm in hardware.It produces a different (but still valid) compressed byte stream, and causes the test failures in ruby/zlib. As a workaround, set the environment variable
DFLTCC=0
disabling the implementation in zlib on s390x to the failing tests.Note we need to test in a child Ruby process with
assert_separately
to test on theDFLTCC=0
set by the parent Ruby process.I tested the ruby/zlib unit test on Ruby's s390x Ubuntu jammy server, and confirmed it passed.
The 2nd commit
I want to use Travis CI again for this repository to test the Ubuntu jammy s390x case on CI. Right now the only choice to test s390x on pull-request is Travis CI as far as I know.