csirac2 / snazzer

btrfs snapshotting and backup system offering snapshot measurement, transport and pruning.
BSD 2-Clause "Simplified" License
89 stars 9 forks source link

tests: make failure message output from bats useful #43

Closed csirac2 closed 7 years ago

csirac2 commented 7 years ago

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 commented 7 years ago

This seems like a very good solution to the no-diagnostic-output-problem. :+1:

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 commented 7 years ago

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 commented 7 years ago

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

florianjacob commented 7 years ago
> sh --version
GNU bash, Version 4.3.46(1)-release (x86_64-unknown-linux-gnu)
florianjacob commented 7 years ago

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? :question: :smile:

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. :wink:

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

csirac2 commented 7 years ago

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.