add "&&" to ensure "status" is included in test pass/fail

[MichaelDimmitt]

Summary:

Core excercise number 3: "Error Handling"

Submitting the solution with all tests giving a result of "status code 0"

Two tests should have failed for this excercise:

@test "incorrect arguments" 
@test "print usage banner with no value given" 

With the addition of "&&" the test suite is correctly failing.

Happy to make alternative changes if preferred.

Additionally,
let me know if you want me to add this to all of the test suites for the bash track.

Below are images where the mentor spotted the excercise should have had a failing test suite.
Cheers, Michael Dimmitt

---

Reviewer Resources:

Track Policies

[IsaacG]

Assuming you're running bats from https://github.com/sstephenson/bats then test suite should be setting set -e which should cause any failed line to trigger a test failure.

Using your first solution,

» BATS_RUN_SKIPPED=true bats *test*
 ✓ correct arguments
 ✓ one long argument
 ✗ incorrect arguments
   (in test file error_handling_test.sh, line 25)
     `(( status == 1 ))' failed
 ✗ print usage banner with no value given
   (in test file error_handling_test.sh, line 33)
     `(( status == 1 ))' failed
 ✓ empty argument

[MichaelDimmitt]

> BATS_RUN_SKIPPED=true bats error_handling_test.sh

> bats error_handling_test.sh 

> bash --version
GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin19)
Copyright (C) 2007 Free Software Foundation, Inc.

> bats -v
Bats 0.4.0

Environment: mac os, catalina.

[MichaelDimmitt]

Even if I add set -e to error_handling_test.sh
the tests are not failing on the incorrect status code.

[IsaacG]

Let's go for a minimal reproduction of the issue.

» tmp=$(mktemp) ; cat << EOF > "$tmp"
@test a {
 false
 true
}
@test b {
 false
}
@test c {
 true
}
EOF

» bats "$tmp"; rm "$tmp"
 ✗ a
   (in test file /tmp/tmp.KI12EzFTeK, line 2)
     `false' failed
 ✗ b
   (in test file /tmp/tmp.KI12EzFTeK, line 6)
     `false' failed
 ✓ c

3 tests, 2 failures

If you get different results than I get, that should be filed as a bats bug.

[MichaelDimmitt]

@IsaacG , same results

Please let me know what else I can do.
Happy to make changes.

And if decided to close the pr thats fine too.

[IsaacG]

You can try and slowly morph test a to match the exercism test, line by line, to figure out which line causes the issue. Start by adding a run false the run script. Start with a script that just runs false and add line by line until one line triggers the issue.

[glennj]

@MichaelDimmitt looks like you're using the unsupported https://github.com/sstephenson/bats -- this repo is known to be buggy and appears to be abandoned.

The recommended bats package is https://github.com/bats-core/bats-core as documented in https://exercism.io/tracks/bash/tests

I would vote to close this PR: having the tests's check separate allows the more specific error to be shown, instead of "the test failed due to either the status or the output"

[MichaelDimmitt]

ok, I think I was using brew install bats but I will see if there is an alternate install available.

thanks for the input.

I will uninstall and try with bats-core 👍

[bkhl]

ok, I think I was using brew install bats but I will see if there is an alternate install available.

Probably would be worthwhile submitting an update/switch to bats-core in Homebrew, so that we don't get students running into this.

I don't use it myself, but maybe someone here can try to submit a change to https://github.com/Homebrew/homebrew-core/blob/master/Formula/bats.rb ?

[MichaelDimmitt]

@glennj , your suggestion did the trick.

Gaving bats installed instead of bats-core was causing the issue.

Thanks @glennj , and @IsaacG

[MichaelDimmitt]

@bkhl, Thanks for the suggestion.

I am going to try submitting the suggested pr to homebrew.

Will post a link to the pr shortly.

[MichaelDimmitt]

@bkhl ,

Homebrew/homebrew-core#27796

It looks like homebrew does not want to update this package because bats has
6.6k stars and bats-core has 1.9k stars.

also I think they need to manually create an alias.
I attempted to create an alias and was not able to test locally using
brew install --build-from-source bats
Homebrew/brew#469 (comment)

I tried to rewrite the ruby formula to do the following

class Bats < BatsCore
end

However it errors out with the following,
Error: bats: uninitialized constant Formulary::FormulaNamespace

I believe this is because homebrew requires all formulas to extend formula
class Bats < Formula

therefore not submitting a pr.

[bkhl]

The project was handed over from the old maintainer to bats-core in this ticket: sstephenson/bats#150 (comment) , so you can refer them to the discussion there.

[MichaelDimmitt]

Created a discussion on homebrew with the information discussed.
Homebrew/discussions#24