Use Bats assertion libraries

[hyperupcall]

Summary

There are several helper libraries to make it easier to write tests and understand failures if they occur.

For example, if [ have = want ] is replaced with assert_equal 'have' 'want' (a function from bats-assert), then the following will be printed.

-- values do not equal --
expected : want
actual   : have
--

This also solves the issue of rogue printing statements that I continue to find during tests.

Dependency Management

Before this is merged, I wanted to inquire about the preferred approach to using this dependency (I am assuming the consensus is that the tradeoff of the dependency is worth the DX benefit it yields)

To get started quickly, I just used my @hyperupcall/bats-all library. Using it is faster than sourcing each individual testing library and I'm trying to get it listed on the bats site. It is also an option to install each package separately (bats-support, bats-file, and bats-assert).

As for managing the dependency, I wasn't sure if git modules or subtrees were preferred, so I went with npm since it seemed like the simplest and cleanest approach.

TODO

• Add to docs in testing section
• Replace more $output checks with bats testing functions
• Replace [ -f checks with bats testing functions
• Ignore (node_modules) through tests, gitignore, checkstyle, if we choose to use npm

[jthegedus]

Interesting. A few initial thoughts on this.

We don't particularly want to add Nodejs ecosystem to our dev dependencies for the core (docs site is a reluctant exception). Adding Nodejs to the core dev experience would ideally be managed by .tool-versions, but I imagine we would hit issues similar to those we had when adding Python to .tool-versions in the repo root for scripts/checkstyle.py. That is, because the default behaviour is to use ~/.asdf/installs/<tool>/<version> to install tools and their versions, when other tools run in those dirs, say when installing tools to <version>/node_modules, they will hit ~/.asdf/.tool-versions from the repo which may cause installation issues.

Anyway, I am very reluctant to use Nodejs just for package distribution of some Shell scripts.

What I would like to see with introducing dependencies like this is managing them with asdf via plugins and .tool-versions.

The questions I then have is around how we utilise those when we're trying to source the files in tests.

And we would have to be certain these extra dependencies don't leak into the core user-executed code.

[hyperupcall]

Anyway, I am very reluctant to use Nodejs just for package distribution of some Shell scripts.
What I would like to see with introducing dependencies like this is managing them with asdf via plugins and .tool-versions.

Hm I see, so I guess this would be related to #367? We wouldn't be managing a language per se, but just some tool.

If we do it this way, ignoring the maintenance burden of having to maintain an asdf plugin for these library utilities, how would we solve the issue you mention - when adding entries to the .tool-versions of this repository other tools will hit the file, like what happened with Python? Because that seems like a blocker, and the only thing I can think of is moving the directory to a different location in a breaking change release (which I don't think is a particularly good idea).

And we would have to be certain these extra dependencies don't leak into the core user-executed code.

I think this asdf plugin for these libraries exposes some source.bash file for us to source (in test/test_helpers.bash), then it will be okay? I guess we would have to create a stub binary to fix the current requirements of making an asdf plugin.

[jthegedus]

like what happened with Python? Because that seems like a blocker, and the only thing I can think of is moving the directory to a different location in a breaking change release

I think using XDG directories as the default would resolve this. But as you mention, definitely a breaking change.

Hm I see, so I guess this would be related to #367? We wouldn't be managing a language per se, but just some tool.

I am not sure how different managing these scripts would be to Bats aside from the fact it has a CLI binary to shim and these won't.

[hyperupcall]

I think using XDG directories as the default would resolve this. But as you mention, definitely a breaking change.

Currently, #1351 doesn't move asdf directories - it only makes using XDG Base Dir-compliant directories the default, and still searches for old directory locations for backwards compatibility. I don't like the idea of moving directories behind a user's back.

I know you said you wanted to use .tool-versions to manage this, but to me it doesn't seem like it's the best tool for the job. What do you think about git submodules or git subtree? It doesn't introduce a NodeJS dependency. The downside to git submodules is that people may forget to add --recursive when cloning or init/update the submodules if they have already cloned asdf and want to run the tests. And of course at least for me submodules is kind of annoying to use sometimes. The downside to git subtree is that subtree doesn't come installed with Git on some distributions (if that is the case, there usually is a git-subtree package to install). Just thinking out loud, since it seems weird that this improvement would be blocked by an issue like #1351

I am not sure how different managing these scripts would be to Bats aside from the fact it has a CLI binary to shim and these won't.

👍

[jthegedus]

Just thinking out loud, since it seems weird that this improvement would be blocked by an issue like #1351

This isn't blocked by #1351. #1351 just unblocks us using managing more tools in our .tool-versions file with an out for users who have conflicts (as with Python)

Submodules and Subtree are a "no" vote from me.

I know you said you wanted to use .tool-versions to manage this, but to me it doesn't seem like it's the best tool for the job

Ideally developers who contribute to this repo only have to add plugins and run asdf install. We've already gone against this twice, once for the docs/ and again for the scripts/checkstyle.py. I don't want to do it a third time, especially when this would affect running scripts/test.bash. The other two times don't stop people running out test suite, this would be a hard requirement.

The larger point to be made here is that these libraries don't introduce much more than syntactic sugar (from my limited understanding). While making it easier to read, people should get the gist of what a test looks like from the hundreds we have in this repo which now have little variation due to the consistent linting/style-checking you have helped setup and enforce. Because of this the cost is high for seemingly little gain.

[hyperupcall]

I see your point - these libraries don't add any major features that we can't live without, and we have more uniformity now in the tests and the code. So there is less to gain, especially since it might be harder to get the dependency installed.

Therefore, I guess I'll close this issue 😅

[jthegedus]

Exactly. We're always open to suggestions, the conversations are useful to have. Thanks for contributing so frequently 🙇

[hyperupcall]

Happy to help! :)

[Stratus3D]

Sorry for the late reply again here. I was going to weigh in a week ago but time got away from me.

Ideally developers who contribute to this repo only have to add plugins and run asdf install. We've already gone against this twice, once for the docs/ and again for the scripts/checkstyle.py. I don't want to do it a third time, especially when this would affect running scripts/test.bash. The other two times don't stop people running out test suite, this would be a hard requirement.

I avoid anything npm/Node/JavaScript like the plague. I'd really hate to have to use npm just to managed out tests written in BATS/Bash. I'm all for better test code too, but these assertions don't seem to be adding much. I have to agree with @jthegedus that this isn't the right approach. Thanks for all your contributions @hyperupcall ! Sorry to reject this one.