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

tests: make failure message output from bats useful #43

Merged
merged 3 commits into from
Nov 6, 2016

Conversation

csirac2
Copy link
Owner

@csirac2 csirac2 commented Nov 5, 2016

Let's use diff -u expected actual rather than [ "$foo" = "bar" ].

This should make bats emit something useful when tests fail rather
than a simple line number in the tests that failed.

Closes #42

Let's use diff -u expected actual rather than [ "$foo" = "bar" ].

This should make bats emit something useful when tests fail rather
than a simple line number in the tests that failed.

Closes #42
@florianjacob
Copy link
Collaborator

This seems like a very good solution to the no-diagnostic-output-problem. 👍

Problem is, I still get exactly the same output as in #33

 ✗ sudo -n snazzer --list-snapshots '--all' 'foo=" some stuff "' 'hel'\\'' squot '\\''lo' 'asd " dquot " fgh' 'ap ple' ' bon'\\''squot'\\''jour' 'there'
   (in test file tests/snazzer-send-wrapper.bats, line 56)                                                                                  
     `[ "$status" = "0" ]' failed

I think I managed to nail down the problem with this:

@test "provocative test" {
    run "/usr/bin/true"
    echo "This is a test."
    [ "0" = "1" ]
    echo "This is another test."
}

results in:

 ✗ provocative test
   (in test file tests/snazzer-send-wrapper.bats, line 57)                                                                                  
     `[ "0" = "1" ]' failed                                                                                                                 
   This is a test. 

So it seems like tests are directly exited after a failed assertion comparison, and the diff is never executed for a failed test.

@florianjacob
Copy link
Collaborator

Moving the status assertion below the diff call for my failing test, I'm getting output!

 ✗ sudo -n snazzer --list-snapshots '--all' 'foo=" some stuff "' 'hel'\\'' squot '\\''lo' 'asd " dquot " fgh' 'ap ple' ' bon'\\''squot'\\''jour' 'there'
   (in test file tests/snazzer-send-wrapper.bats, line 65)                                                                                  
     `diff -u $(expected_file) $(actual_file)' failed                                                                                       
   --- /tmp/snazzer-tests/snazzer-send-wrapper.bats_7.expected  2016-11-06 00:40:51.458084026 +0100                                         
   +++ /tmp/snazzer-tests/snazzer-send-wrapper.bats_7.actual    2016-11-06 00:40:51.478084354 +0100                                         
   @@ -1 +1 @@                                                                                                                              
   -10                                                                                                                                      
   +/tmp/snazzer-tests/bin/snazzer-send-wrapper (for ) REJECTED: sudo -n snazzer --list-snapshots '--all' 'foo=" some stuff "' 'hel'\\'' squot '\\''lo' 'asd " dquot " fgh' 'ap ple' ' bon'\\''squot'\\''jour' 'there' ### REASON: This should never happen, stopped: '--all' 'foo=" some stuff "' 'hel'\\'' squot '\\''lo' 'asd " dquot " fgh' 'ap ple' ' bon'\\''squot

@csirac2
Copy link
Owner Author

csirac2 commented Nov 6, 2016

Cool! What provides /bin/sh on your system? I'll amend the branch to move the exit status assertions after the diffs.

When exit status isn't what we expect, usually the diff will show
some clues as to what's going wrong more than a failed status check.
@florianjacob
Copy link
Collaborator

> sh --version
GNU bash, Version 4.3.46(1)-release (x86_64-unknown-linux-gnu)

@florianjacob
Copy link
Collaborator

What provides /bin/sh on your system?

Did you notice something how this could result in the test failure, or why did you want to know? ❓ 😄

It might be relevant that while /bin/sh is a bash, my login shell is actually not bash compatible (fish), so e.g. any shell script that's not properly shebanged will fail. 😉

If there's nothing more you want to do, I think this would be ready to merge. 👍 for managing to add this feature with almost no added complexity.

@csirac2
Copy link
Owner Author

csirac2 commented Nov 6, 2016

Indeed - I am torn between providing convenience methods for some of this stuff, and keeping it obvious in the tests... fixtures.sh really needs some refactoring, I don't know what I was thinking when I wrote all that.

As for why it's failing with bash: well, I tried very hard to avoid introducing any bash-isms so that alternate shells would work happily. And yet, here we are! I wrote it for dash, the default on Debian, and it seems I've done something that isn't properly emulated by bash in /bin/sh personality.

Will address the "fails with bash" problem on #33, thanks again for reviewing.

@csirac2 csirac2 merged commit 2119c39 into master Nov 6, 2016
@csirac2 csirac2 deleted the improve-test-failure-output branch November 6, 2016 10:27
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

Successfully merging this pull request may close these issues.

2 participants