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.

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

add bats-assert and bats-file plugins to bats test suite #536

Closed
MichaelDimmitt opened this issue Aug 27, 2021 · 5 comments · Fixed by #539
Closed

add bats-assert and bats-file plugins to bats test suite #536

MichaelDimmitt opened this issue Aug 27, 2021 · 5 comments · Fixed by #539

Comments

@MichaelDimmitt
Copy link

I created a request in bats-core to add homebrew support for their plugins using homebrew taps.

bats-core/bats-core#492

In the meantime,
https://github.com/bats-core/bats-assert

Refers to
https://github.com/ztombol/bats-docs

Which uses the homebrew tap from kaos
https://github.com/kaos/homebrew-shell

$ brew tap kaos/shell
$ brew install bats-assert
$ brew install bats-file

then in every test you have to load the files.

load '/opt/homebrew/lib/bats-support/load.bash'
load '/opt/homebrew/lib/bats-assert/load.bash'

@test 'refute_output()' {
  run echo 'want'
  refute_output 'want'
}
@test 'assert_equal()' {
  assert_equal 'have' 'hav'
}

Screen Shot 2021-08-26 at 8 57 52 PM

@glennj
Copy link
Contributor

glennj commented Aug 27, 2021

I've thought about this. I know they contain some cool stuff,
but I don't think it's really worth the overhead for students to have to install them.

Not only the overhead of installing them, but not everyone will (be able to) install them into /opt/homebrew. What about folks on Windows. We'd have to instruct people to set an environment variable holding the path to the installation point so the test file can start with something like

load "$BATS_LIB_HOME"/bats-support/load.bash
load "$BATS_LIB_HOME"/bats-assert/load.bash

I think that could be tricky to explain to people who are just learning bash.

Other views? @guygastineau @kotp @IsaacG

@MichaelDimmitt
Copy link
Author

As a note: I have created issues in bats-core requesting updates to have them publish an npm package or homebrew packages for their plugins alongside the main bats-core repo.
bats-core/bats-core#492
bats-core/bats-core#493

I had a way to install the old version of bats via npm
https://gist.github.com/MichaelDimmitt/1f1a530b1c68cac26a26fccd890a4bb7

But it looks like the npm install for bats-core is currently not working
https://github.com/bats-core/bats-core/packages/611449
npm install @bats-core/[email protected] - unless I need to add github registry to my npm.

@glennj
Copy link
Contributor

glennj commented Aug 27, 2021

An additional note, the list-ops test script doesn't use run. While it would be nicer with assert_equal, it's OK without.

@kotp
Copy link
Member

kotp commented Aug 28, 2021

I think there is definitely value in keeping the locally running test suite simple. It is kind of irritating to need to have node or a package system in order to install a system for testing bash. A simple assertion program that can evaluate if there are matches is often fine for testing, and simple if it is the same language as being taught. Less barrier to entry.

@glennj
Copy link
Contributor

glennj commented Sep 3, 2021

As I commented in #531, this has become more important for students coding in the online browser. Having the only test feedback as "the output failed" is no good.

What I did in the Tcl track is put a "testHelpers" file in every exercise, and the ".test" file will source it. That keeps the test file clean, but also makes the extra functionality available.

I propose to do the same thing for bats-assert: the source files for bats-assert (and the required bats-support) are small, so the can just be concatenated into a single file to be sourced.

A draft PR is forthcoming soon.

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 a pull request may close this issue.

3 participants