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.

Sign up for GitHub

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

Jump to bottom

Require STDIN to be specified explicitly with - #14

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Require STDIN to be specified explicitly with - #14

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1a75841
stdin must be specified explicitly with `-`
jasonkarns Jun 16, 2016
b4e90ad
drop early return
jasonkarns Jun 16, 2016
0fb9d2a
docs
jasonkarns Jun 18, 2016
ae447cd
Update code comments
jasonkarns Nov 8, 2016
0dc0b49
Allow longform --stdin option as well
jasonkarns Nov 8, 2016
581aa70
Document stdin option in readme
jasonkarns Nov 22, 2016
adc1c7b
Remove leftover debugging lines
jasonkarns Nov 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,13 @@ By default, literal matching is performed. The assertion fails if
}
```

The expected output can be specified with a heredoc or standard input as well.
The expected output can be specified with a heredoc or standard input as well,
by providing `-` as an option.
Copy link
Owner

Choose a reason for hiding this comment

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

Could you mention the long option in the README as well. Like we did for other options:

Partial matching can be enabled with the --partial option (-p for short). When used, the assertion fails if the expected substring is not found in $output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added! 581aa70


```bash
@test 'assert_output() with pipe' {
run echo 'have'
echo 'want' | assert_output
echo 'want' | assert_output -
}
```

Expand Down Expand Up @@ -287,12 +288,13 @@ By default, literal matching is performed. The assertion fails if
}
```

-The unexpected output can be specified with a heredoc or standard input as well.
The unexpected output can be specified with a heredoc or standard input as well,
by providing `-` as an option.
Copy link
Owner

Choose a reason for hiding this comment

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

Could you mention the long option in the README as well. Like we did for other options:

Partial matching can be enabled with the --partial option (-p for short). When used, the assertion fails if the expected substring is not found in $output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added! 581aa70


```bash
@test 'refute_output() with pipe' {
run echo 'want'
echo 'want' | refute_output
echo 'want' | refute_output -
}
```

Expand Down
26 changes: 19 additions & 7 deletions src/assert.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,9 @@ assert_failure() {
# Options:
# -p, --partial - partial matching
# -e, --regexp - extended regular expression matching
# -, --stdin - read expected output from the standard input
# Arguments:
# $1 - [=STDIN] expected output
# $1 - expected output
# Returns:
# 0 - expected matches the actual output
# 1 - otherwise
Expand All @@ -185,12 +186,14 @@ assert_failure() {
assert_output() {
Copy link
Owner

Choose a reason for hiding this comment

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

Could you update the function comment too? For example:

 # Options:
 #   -p, --partial - partial matching
 #   -e, --regexp - extended regular expression matching
+#   - - read expected output from the standard input
 # Arguments:
-#   $1 - [=STDIN] expected output
+#   $1 - expected output
 # Returns:
 #   0 - expected matches the actual output
 #   1 - otherwise

local -i is_mode_partial=0
local -i is_mode_regexp=0
local -i use_stdin=0

# Handle options.
while (( $# > 0 )); do
case "$1" in
-p|--partial) is_mode_partial=1; shift ;;
-e|--regexp) is_mode_regexp=1; shift ;;
-|--stdin) use_stdin=1; shift ;;
--) shift; break ;;
*) break ;;
esac
Expand All @@ -205,17 +208,19 @@ assert_output() {

# Arguments.
local expected
(( $# == 0 )) && expected="$(cat -)" || expected="$1"
if (( use_stdin )); then
expected="$(cat -)"
else
expected="$1"
fi

# Matching.
if (( is_mode_regexp )); then
if [[ '' =~ $expected ]] || (( $? == 2 )); then
echo "Invalid extended regular expression: \`$expected'" \
| batslib_decorate 'ERROR: assert_output' \
| fail
return $?
fi
if ! [[ $output =~ $expected ]]; then
elif ! [[ $output =~ $expected ]]; then
batslib_print_kv_single_or_multi 6 \
'regexp' "$expected" \
'output' "$output" \
Expand Down Expand Up @@ -265,8 +270,9 @@ assert_output() {
# Options:
# -p, --partial - partial matching
# -e, --regexp - extended regular expression matching
# -, --stdin - read unexpected output from the standard input
# Arguments:
# $1 - [=STDIN] unexpected output
# $1 - unexpected output
# Returns:
# 0 - unexpected matches the actual output
# 1 - otherwise
Expand All @@ -278,12 +284,14 @@ assert_output() {
refute_output() {
Copy link
Owner

Choose a reason for hiding this comment

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

Could you update the function comment too? For example:

 # Options:
 #   -p, --partial - partial matching
 #   -e, --regexp - extended regular expression matching
+#   - - read unexpected output from the standard input
 # Arguments:
-#   $1 - [=STDIN] unexpected output
+#   $1 - unexpected output
 # Returns:
 #   0 - unexpected matches the actual output
 #   1 - otherwise

local -i is_mode_partial=0
local -i is_mode_regexp=0
local -i use_stdin=0

# Handle options.
while (( $# > 0 )); do
case "$1" in
-p|--partial) is_mode_partial=1; shift ;;
-e|--regexp) is_mode_regexp=1; shift ;;
-|--stdin) use_stdin=1; shift ;;
--) shift; break ;;
*) break ;;
esac
Expand All @@ -298,7 +306,11 @@ refute_output() {

# Arguments.
local unexpected
(( $# == 0 )) && unexpected="$(cat -)" || unexpected="$1"
if (( use_stdin )); then
unexpected="$(cat -)"
else
unexpected="$1"
fi

if (( is_mode_regexp == 1 )) && [[ '' =~ $unexpected ]] || (( $? == 2 )); then
echo "Invalid extended regular expression: \`$unexpected'" \
Expand Down
14 changes: 12 additions & 2 deletions test/50-assert-15-assert_output.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,19 @@ load test_helper
[ "${lines[3]}" == '--' ]
}

@test 'assert_output(): reads <expected> from STDIN' {
@test 'assert_output() - : reads <expected> from STDIN' {
Copy link
Owner

Choose a reason for hiding this comment

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

Please factor out common parts of tests to avoid duplicating code. Like it's done here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually have a strong dislike of that extraction. Generally I favor non-dry tests because tests should be explicit and self-contained. In this case (and with test_r_regexp), it's quite difficult to understand what's going on with the extracted function. Instead of the test functioning as example usage, we now have variables (unnamed vars at that) being used in place of the very thing that makes the test unique.

If anything should be extracted, I'd extract the duplicated assertions from all tests:

  [ "$status" -eq 0 ]
  [ "${#lines[@]}" -eq 0 ]

The implementation of those statements are not germane to the test, so abstracting them away can improve readability. However, extracting away the actual invocation is a bad idea, IMO. The unique invocation is literally the raison d'être for the test(s).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened a PR to this PR to show usage of just the assertion helper rather than attempting to dry up the entire test(s).

bats-core#1

run echo 'a'
run assert_output <<STDIN
run assert_output - <<STDIN
a
STDIN
echo "$output"
Copy link
Owner

Choose a reason for hiding this comment

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

Leftover debug print.

[ "$status" -eq 0 ]
[ "${#lines[@]}" -eq 0 ]
}

@test 'assert_output() --stdin : reads <expected> from STDIN' {
run echo 'a'
run assert_output --stdin <<STDIN
a
STDIN
echo "$output"
Copy link
Owner

Choose a reason for hiding this comment

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

Can you remove this debug print left over from a previous PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done adc1c7b

Expand Down
15 changes: 12 additions & 3 deletions test/50-assert-16-refute_output.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,18 @@ load test_helper
[ "${lines[2]}" == '--' ]
}

@test 'refute_output(): reads <unexpected> from STDIN' {
run echo 'a'
run refute_output <<INPUT
@test 'refute_output() - : reads <unexpected> from STDIN' {
run echo '-'
Copy link
Owner

@ztombol ztombol Sep 25, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Why did you change the run command? As far as I can tell, it worked with run echo 'a' as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a stronger assertion that - as input on stdin doesn't cause problems. (Since the - is also the flag to the assertion)

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, okay got it. Thanks!

run refute_output - <<INPUT
b
INPUT
[ "$status" -eq 0 ]
[ "${#lines[@]}" -eq 0 ]
}

@test 'refute_output() --stdin : reads <unexpected> from STDIN' {
run echo '--stdin'
run refute_output --stdin <<INPUT
b
INPUT
[ "$status" -eq 0 ]
Expand Down