Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

batman test fails on Linux #19

Open
lilyball opened this issue Apr 26, 2020 · 6 comments
Open

batman test fails on Linux #19

lilyball opened this issue Apr 26, 2020 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@lilyball
Copy link

Running the batman test suite fails on Linux with signal 141 (SIGPIPE). It appears that batman --version | head -n1 causes batman to fail on a printf call.

@lilyball
Copy link
Author

Same issue with the prettybat test too.

@eth-p
Copy link
Owner

eth-p commented Apr 27, 2020

Thanks for the heads up! I'll check it out and try to fix it soon.

Update:
This doesn't appear to be consistently reproducible (race condition?), and it might take a while to figure out.
image

@eth-p eth-p self-assigned this Apr 27, 2020
@eth-p eth-p added the bug Something isn't working label Apr 27, 2020
@eth-p
Copy link
Owner

eth-p commented Apr 27, 2020

After some research, it seems that head likes to send SIGPIPE when it's done reading, which Bash is more than happy to consider a fatal error with set -o pipefail. I'm surprised I didn't encounter this when writing the tests, but I believe it should be fixed now with e79e747.

Try running ./test.sh --verbose --compiled --suite prettybat a few times (I did 50 to make sure, but 15 or so would probably work), and let me know if that solved your issue.

@sharkdp
Copy link
Collaborator

sharkdp commented Apr 27, 2020

it seems that head likes to send SIGPIPE when it's done reading,

more precisely, when head is done reading, it closes its STDIN, which is connected to your STDOUT. The kernel then sends SIGPIPE to your process if you are trying to write into the closed pipe. This often needs to be handled correctly. In bat, we simply exit with success (code 0): https://github.com/sharkdp/bat/blob/38c096bf7908f762fa1a79a696f1212e52651e36/src/error.rs#L19-L23

@lilyball
Copy link
Author

I ran the batman and prettybat test suites 20 times each and got zero failures, so I think it worked.

More generally, either ignoring the return code of printf or exiting cleanly if it reports code 141 would be better.

@eth-p
Copy link
Owner

eth-p commented Apr 27, 2020

I ran the batman and prettybat test suites 20 times each and got zero failures, so I think it worked.

Good to know!

More generally, either ignoring the return code of printf

The issue wasn't so much the printf call from inside the bat* script, as much as it was the testing framework which used set -e along with set -o pipefail, which—as I just learned—doesn't play nicely with head. For the scripts themselves, I think it would be reasonable to expect them to fail with a nonzero exit if they can't print an error message.

or exiting cleanly if it reports code 141 would be better.

The testing framework doesn't support this feature yet, but it's definitely something I should be thinking about adding support for.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants