Skip to content

add bats-assert functionality #539

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

Merged
merged 10 commits into from
Sep 4, 2021
Merged

add bats-assert functionality #539

merged 10 commits into from
Sep 4, 2021

Conversation

glennj
Copy link
Contributor

@glennj glennj commented Sep 3, 2021

Resolves #536. Resolves #531

The bats-extra.bash will be added to every exercise. The contents are the src files for bats-support and bats-assert concatenated, with comment blocks removed.

Each exercise's test file will be updated to use bats-assert functions.

The augmented test failure output is important particularly for the


Reviewer Resources:

Track Policies

@glennj
Copy link
Contributor Author

glennj commented Sep 3, 2021

For example, "hello world":

The successful test output appearance does not change

$ bats hello_world_test.sh
 ✓ Say Hi!

1 test, 0 failures

If the exit status is non-zero:

$ cat hello_world.sh
#!/usr/bin/env bash

echo "Hello, World!"
exit 2

$ bats hello_world_test.sh
 ✗ Say Hi!
   (from function `assert_success' in file ./bats-extra.bash, line 409,
    in test file hello_world_test.sh, line 10)
     `assert_success' failed with status 2

   -- command failed --
   status : 2
   output : Hello, World!
   --


1 test, 1 failure

$ echo $?
1

If the output does not match what's expected:

$ cat hello_world.sh
#!/usr/bin/env bash

echo "Goodbye, Mars!"

$ bats hello_world_test.sh
 ✗ Say Hi!
   (from function `assert_output' in file ./bats-extra.bash, line 394,
    in test file hello_world_test.sh, line 13)
     `assert_output "Hello, World!"' failed

   -- output differs --
   expected : Hello, World!
   actual   : Goodbye, Mars!
   --


1 test, 1 failure

$ echo $?
1

@glennj
Copy link
Contributor Author

glennj commented Sep 3, 2021

@exercism/bash Thoughts please? This will turn into a very substantial PR with every exercise being affected. For cleanliness, I'll make a commit that's just adding the new file, and another with changes to the test files.

Following that, there are also changed to the bash-test-runner to make.

Copy link
Member

@kotp kotp left a comment

Choose a reason for hiding this comment

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

Looks good to me, with one suggestion for the repeated "kv" use in the names to show what the placeholder might be.

@@ -1,10 +1,14 @@
#!/usr/bin/env bash
source ./bats-extra.bash
Copy link
Member

Choose a reason for hiding this comment

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

Any test to fail if the file is missing or cannot be sourced?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, well if it can't be found then exercism download would be broken. If the student accidentally deletes it, then a helpful message to re-download would be useful

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 changed source to the bats function load. If the file does not exist, then

$ bats hello_world_test.sh
 ✗ File hello_world_test.sh
bats: bats-extra.bash does not exist
   (in test file hello_world_test.sh, line 9)
     `load bats-extra.bash' failed

1 test, 1 failure

@glennj glennj requested review from IsaacG and kotp September 3, 2021 20:31
@kotp
Copy link
Member

kotp commented Sep 3, 2021

I can see that it is titled WIP, but "convert to draft" is probably a better solution. It can prevent premature merges.

@glennj glennj changed the title WIP: add bats-assert functionality add bats-assert functionality Sep 4, 2021
@glennj
Copy link
Contributor Author

glennj commented Sep 4, 2021

Phew, all tests passing.

@glennj
Copy link
Contributor Author

glennj commented Sep 4, 2021

I can see that it is titled WIP, but "convert to draft" is probably a better solution. It can prevent premature merges.

No longer WIP/draft

@glennj glennj requested review from kotp and IsaacG September 4, 2021 02:17
@glennj
Copy link
Contributor Author

glennj commented Sep 4, 2021

Reviewers: coments about the commits here:

commit msg notes
dd41d86 WIP: add bats-assert functionality convert "hello world"
646c5fd use bats "load" function instead of "source" errors out if bats-extra.bash not source-able
05954bd add bats-extra.bash to all exercises adding file to all other exercises
ebd194c transform test scripts to use bats-assert functions this is big delta and where you'll want to spend your time: convert all test suites to use assert functions
5ae639f with an associative array, "${map[@]}" order is indeterinate, broke i… fix to example script for bash 5.1
f2fc14c in arithmetic, bare map entry with quote in key broke in bash 5.1 fix to example script for bash 5.1
54d7be2 update workflow, using github cli with auto-pagination, to pull in AL… update GHA workflow script to work with loads of changes
2495798 add env var to workflow for auth update GHA workflow script to work with loads of changes

@glennj glennj merged commit 21266ba into exercism:main Sep 4, 2021
@MichaelDimmitt
Copy link

MichaelDimmitt commented Sep 5, 2021

cool, thanks for adding this.

nice idea to save users from needing to install via npm, homebrew, submodule or otherwise.
since the change is posix compliant we can just add the files for them.

and adding the extra file bats-extra.bash that
concatenated bats assert and bats support

In the future maybe a script iterates through all files in the test folder and updates bats-extra.bash if the plugins have an update we want to add. But no need to overengineer at present moment.

definately gives more output to students 👍

@glennj glennj deleted the adding-bats-assert-functionality branch September 5, 2021 01:34
@glennj
Copy link
Contributor Author

glennj commented Sep 5, 2021

Thanks for the inspiration. While I was working on it I was thinking I should apologize: I think I was pretty dismissive about your ideas. So, sorry. Working with the v3 website really revealed the need for better test failure feedback.

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.

add bats-assert and bats-file plugins to bats test suite Add diffing support to bash tests
5 participants