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

Test fails on s390x #218

Closed
misery opened this issue May 18, 2020 · 8 comments
Closed

Test fails on s390x #218

misery opened this issue May 18, 2020 · 8 comments

Comments

@misery
Copy link

misery commented May 18, 2020

Describe the bug
Tried to build a package for Alpine. It works without problems on my machine but it fails on s390x CI.
https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/7981

To Reproduce
Steps to reproduce the behavior:

  1. Build croc for s390x
  2. Run unit tests

Expected behavior
No failing tests. ;-)

Version
8.0.11

Additional context
https://gitlab.alpinelinux.org/misery/aports/-/jobs/118553

>>> croc: Unpacking /var/cache/distfiles/croc_8.0.11_src.tar.gz...
?   	github.com/schollz/croc/v8	[no test files]
?   	github.com/schollz/croc/v8/src/cli	[no test files]
ok  	github.com/schollz/croc/v8/src/comm	0.319s
ok  	github.com/schollz/croc/v8/src/compress	0.016s
ok  	github.com/schollz/croc/v8/src/croc	2.752s
ok  	github.com/schollz/croc/v8/src/crypt	0.017s
?   	github.com/schollz/croc/v8/src/install	[no test files]
ok  	github.com/schollz/croc/v8/src/message	0.429s
?   	github.com/schollz/croc/v8/src/models	[no test files]
ok  	github.com/schollz/croc/v8/src/tcp	0.471s
[172.17.0.4] <nil>
--- FAIL: TestIMOHashFile (0.14s)
    utils_test.go:68: 
        	Error Trace:	utils_test.go:68
        	Error:      	Not equal: 
        	            	expected: "c0d1e123a96a598ea801cc503d3db8c0"
        	            	actual  : "c0d1e123a419ea942bc9b06c97cfb64e"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-c0d1e123a96a598ea801cc503d3db8c0
        	            	+c0d1e123a419ea942bc9b06c97cfb64e
        	Test:       	TestIMOHashFile
--- FAIL: TestHashFile (0.00s)
    utils_test.go:164: 
        	Error Trace:	utils_test.go:164
        	Error:      	Not equal: 
        	            	expected: "18c9673a4bb8325d323e7f24fda9ae1e"
        	            	actual  : "1860fd5aeb1ac95051d1cc57840037a1"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-18c9673a4bb8325d323e7f24fda9ae1e
        	            	+1860fd5aeb1ac95051d1cc57840037a1
        	Test:       	TestHashFile
148.100.88.62
172.17.0.4
FAIL
FAIL	github.com/schollz/croc/v8/src/utils	1.025s
FAIL
>>> ERROR: croc: check failed
@schollz
Copy link
Owner

schollz commented May 18, 2020

@misery Okay, my bet is that it has something to the generation of a random file that was used for the tests. I generated a random file with seed 0 on my computer, which are the hashes I precomputed. Perhaps generating a random file from seed 0 is not the same for s390x? In any case, I changed the tests to work on a reproducible file filled with the same character. Can you try it now?

@misery
Copy link
Author

misery commented May 18, 2020

Thanks for fast reply. :-)

But it's still broken: https://gitlab.alpinelinux.org/misery/aports/-/jobs/118672

By the way... armv7 crashes now.
https://gitlab.alpinelinux.org/misery/aports/-/jobs/118675

Perhaps generating a random file from seed 0 is not the same for s390x?

Well, I don't know s390x. :-) I just saw the failure in Alpines CI.

@misery
Copy link
Author

misery commented May 20, 2020

Looks like mips64 is also broken.
https://build.alpinelinux.org/buildlogs//build-edge-mips64/testing/croc/croc-8.0.11-r0.log

Is it a flaky test or is the test environment broken?

@theStack
Copy link
Contributor

My guess here would be that those failing tests are caused by endianness problems, as the common denominator for the affected architectures (s390x, mips64) is that they use big endian byte ordering compared to the much more common little endian. Maybe imohash is not endian-agnostic?

@schollz schollz closed this as completed Apr 13, 2021
@misery
Copy link
Author

misery commented Apr 14, 2021

It is still failing with 8.6.12 on s390x and mips/mips64

?   	github.com/schollz/croc/v8	[no test files]
?   	github.com/schollz/croc/v8/src/cli	[no test files]
ok  	github.com/schollz/croc/v8/src/comm	0.387s
ok  	github.com/schollz/croc/v8/src/compress	0.147s
ok  	github.com/schollz/croc/v8/src/croc	2.942s
ok  	github.com/schollz/croc/v8/src/crypt	0.022s
?   	github.com/schollz/croc/v8/src/install	[no test files]
ok  	github.com/schollz/croc/v8/src/message	0.493s
?   	github.com/schollz/croc/v8/src/models	[no test files]
ok  	github.com/schollz/croc/v8/src/tcp	0.761s
[172.17.0.2] <nil>
--- FAIL: TestHashFile (0.00s)
    utils_test.go:169: 
        	Error Trace:	utils_test.go:169
        	Error:      	Not equal: 
        	            	expected: "18c9673a4bb8325d323e7f24fda9ae1e"
        	            	actual  : "1860fd5aeb1ac95051d1cc57840037a1"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-18c9673a4bb8325d323e7f24fda9ae1e
        	            	+1860fd5aeb1ac95051d1cc57840037a1
        	Test:       	TestHashFile
148.100.88.62
172.17.0.2
FAIL
FAIL	github.com/schollz/croc/v8/src/utils	0.480s
FAIL

https://gitlab.alpinelinux.org/misery/aports/-/jobs/371363

@kalafut
Copy link

kalafut commented Apr 20, 2021

Maybe imohash is not endian-agnostic?

@theStack @misery Given this issue around the murmur3 hash used within imohash, I think that's a reasonable assumption. I'd been considering swapping the murmur3 implementation to something like this one from twmb which claims stable behavior across endianess. This is a good nudge.

@kalafut
Copy link

kalafut commented Apr 20, 2021

I just pushed github.com/kalafut/[email protected] which uses the alternate hashing library and should be endian-agnostic. The croc tests all pass fine without change. I don't have a good way to test if this does really fix the endianness issue though (but it should).

kalafut pushed a commit to kalafut/croc that referenced this issue Apr 21, 2021
This may help with big-ending hashing issues (e.g. schollz#218). It is also
more memory efficient if full file hashing is used with imohash.
@misery
Copy link
Author

misery commented Apr 22, 2021

@kalafut Thanks, it seems to work.... https://gitlab.alpinelinux.org/misery/aports/-/jobs/377230

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

No branches or pull requests

4 participants