Skip to content
This repository was archived by the owner on Apr 29, 2021. It is now read-only.

Print failed statement in backtrace #57

Merged
merged 2 commits into from
Aug 12, 2014
Merged

Print failed statement in backtrace #57

merged 2 commits into from
Aug 12, 2014

Conversation

ahippo
Copy link
Contributor

@ahippo ahippo commented May 21, 2014

Update tests accordingly.

Please, note that it's quite similar to pull request #15

@ahippo ahippo mentioned this pull request May 21, 2014
Closed
sstephenson
sstephenson reviewed Jun 1, 2014
View reviewed changes
libexec/bats-exec-test
if [ $index -eq 1 ]; then
line="$BATS_LINE_NUMBER"
echo -n "# ("
echo "# (command"
echo "# \`$(sed -e "${line}!d" -e "${line}s:^[ \t]\+::" -e "${line}q" "${filename}")'"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GNU sed appears to accept + here without specifying the extended regex option, but it's non-portable and doesn't work on BSD/OS X sed.

Replace with something like:
sed -e "${line}!d" -e "${line}s:^[ \t][ \t]*::" -e "${line}q" "${filename}"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll fix that.

@sstephenson
Copy link
Owner

Can we stick with the error output format implemented in #15?

@sstephenson sstephenson mentioned this pull request Jun 2, 2014
Closed
@sstephenson sstephenson added this to the 1.0 milestone Jun 2, 2014
@ahippo
Copy link
Contributor Author

ahippo commented Jun 5, 2014

Can we stick with the error output format implemented in #15?

It's fine, I can try to switch to it.
I see, you've done some refactoring of bats_print_stack_trace() in double-brackets branch (49f533e), so I believe, I'd better wait until it's merged to master.

ahippo added 2 commits June 29, 2014 00:30
@ahippo
Print failed statement in backtrace
bbaf0f5 
Update tests accordingly.
@ahippo
Fix backtrace sed script for non-GNU seds
03c6ab4 
Tested on GNU `sed --posix`.

From `info sed`:
`\+'
     As `*', but matches one or more.  It is a GNU extension.

`\CHAR'
     Matches CHAR, where CHAR is one of `$', `*', `.', `[', `\', or `^'.
     Note that the only C-like backslash sequences that you can
     portably assume to be interpreted are `\n' and `\\'; in particular
     `\t' is not portable, and matches a `t' under most implementations
     of `sed', rather than a tab character.
@ahippo
Copy link
Contributor Author

ahippo commented Jun 29, 2014

I've changed the output as suggested and fixed sed command for non-GNU seds.
I've pushed it to ahippo/bats:print-failed-command-v2 (bbaf0f5 and 03c6ab4).
How should I proceed:

  1. rewrite print-failed-command branch
  2. close this pull request and open another one
    ?

@dwijnand
Copy link

ping @sstephenson ^^

@ctapobep
Copy link

Too useful to keep it unmerged :)

@sstephenson
Copy link
Owner

Hi @ahippo. Please just push the new commits to your print-failed-command branch. GitHub will automatically update this pull request. No need to rewrite the branch or open a second PR.

@ahippo
Copy link
Contributor Author

ahippo commented Aug 11, 2014

Hi @ahippo. Please just push the new commits to your print-failed-command branch. GitHub will automatically update this pull request. No need to rewrite the branch or open a second PR.

Done.

@sstephenson sstephenson merged commit 03c6ab4 into sstephenson:master Aug 12, 2014
@sstephenson
Copy link
Owner

Thanks for your work.

This is now merged into master, with some riffing, and a caveat: Bats will display the outermost failing command rather than the innermost failing command.

The outermost command—in other words, the line in the test case function itself—is likely more descriptive than the innermost command, which is often a helper function nested several calls deep.

Consider this example from rbenv's tests. Here's the innermost failing command:

 ✗ resolves symlinks
   (from function `assert_equal' in file test/test_helper.bash, line 51,
    from function `assert_output' in file test/test_helper.bash, line 60,
    from function `assert_success' in file test/test_helper.bash, line 35,
    in test file test/hooks.bats, line 64)
     `} | flunk' failed
   expected: TEST_DIR/home/hola.bash
   actual:   /privateTEST_DIR/home/hola.bash

And here's the outermost failing command:

 ✗ resolves symlinks
   (from function `assert_equal' in file test/test_helper.bash, line 51,
    from function `assert_output' in file test/test_helper.bash, line 60,
    from function `assert_success' in file test/test_helper.bash, line 35,
    in test file test/hooks.bats, line 64)
     `assert_success "${HOME}/hola.bash"' failed
   expected: TEST_DIR/home/hola.bash
   actual:   /privateTEST_DIR/home/hola.bash

The outermost command captures the intent of the test, while the innermost command is an implementation detail.

@ahippo
Copy link
Contributor Author

ahippo commented Aug 12, 2014

Thank you very much for merging the pull request!

For the example from rbenv's tests your change from innermost to outermost failing command seems very natural.
I'll take a look at how it feels on my tests and post here if I have any objections.

@ahippo ahippo deleted the print-failed-command branch August 12, 2014 22:41
duggan pushed a commit to duggan/bats that referenced this pull request Aug 13, 2014
Update line numbers to reflect changes to output introduced by ssteph…
6eeedc7 
…enson#57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
needs review
Projects
None yet
Milestone
1.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ahippo @sstephenson @dwijnand @ctapobep