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

Support Bash "strict mode" (and fix one unit test) #239

Merged
merged 3 commits into from
Dec 24, 2023

Conversation

djpohly
Copy link
Contributor

@djpohly djpohly commented Dec 21, 2023

📚 Description

Some script authors like to use "unofficial Bash strict mode", enough so that supporting it shows up in comparisons of shell testing frameworks.

Case in point: enabling the -u option uncovered a bug in test_unsuccessful_assert_is_directory_not_writable that is fixed in this PR.

🔖 Changes

  • bashunit and its tests now have set -euo pipefail enabled.
  • The test mentioned above has been fixed ($a_file was supposed to be $a_directory).
  • Arithmetic expansions ((...)), which exit 1 if their result happens to be zero, are fixed with || true.
  • The call to check_duplicate_functions does not appear to be intended to cause the script to fail, so it is also silenced with || true.
  • Optional parameters are given default-empty behavior using e.g. ${1-}.
  • Optional variables are assigned defaults by evaluating "${varname=default}".
  • Existence of optional functions is tested explicitly before attempting to execute them. This has the side benefit of ensuring that an external program with the right name (e.g. set_up) will not be run instead.

✅ To-do list

  • I updated the CHANGELOG.md to reflect the new feature or fix
  • I updated the documentation to reflect the changes (Not sure there's anything to document, since this fix doesn't require users to do anything different.)

A few different kinds of changes are made:

  - Optional parameters/variables are changed to ${var-} to default to
    empty or given defaults with ${var=...}.
  - ((...)) arithmetic evaluation "fails" when the result happens to be
    zero, so silence this with "|| true"
This will help ensure that bashunit won't cause problems with tests that
use "set -euo pipefail"

Watermelon AI Summary

AI Summary deactivated by djpohly

GitHub PRs

bashunit is an open repo and Watermelon will serve it for free.
🍉🫶

Copy link
Member

@Chemaclass Chemaclass left a comment

Choose a reason for hiding this comment

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

Wow, this is pretty cool. Thanks!

@Chemaclass Chemaclass self-requested a review December 22, 2023 09:49
@Chemaclass
Copy link
Member

It seems that it is not working as expected for macos... I will check it out when I find some time these days

@Chemaclass Chemaclass changed the base branch from main to strictmode December 24, 2023 16:25
@Chemaclass
Copy link
Member

I will merge this branch into an internal one with the same name strictmode, so I can check it out and play around to reproduce the bug easier, while keeping the original commits from @djpohly 👍🏼

@Chemaclass Chemaclass merged commit 14d2f0e into TypedDevs:strictmode Dec 24, 2023
2 of 7 checks passed
@Chemaclass Chemaclass mentioned this pull request Dec 24, 2023
1 task
@Chemaclass Chemaclass mentioned this pull request Jul 12, 2024
1 task
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.

2 participants