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

Foundation for strictmode #241

Merged
merged 9 commits into from
Jul 12, 2024
Merged

Foundation for strictmode #241

merged 9 commits into from
Jul 12, 2024

Conversation

Chemaclass
Copy link
Member

@Chemaclass Chemaclass commented Dec 24, 2023

📚 Description

Original PR: #239 by @djpohly

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

  • 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}".

🏗️ Follow up

  • Add set -euo pipefail to all tests. Currently not possible; needs some more investigation [wip]

✅ To-do list

  • I updated the CHANGELOG.md to reflect the new feature or fix

djpohly and others added 5 commits December 21, 2023 16:22
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"
Support Bash "strict mode" (and fix one unit test)
 Conflicts:
	CHANGELOG.md
	src/console_results.sh
	src/env_configuration.sh
	src/runner.sh
for some reason, having this set in all test files make some tests to break, and this needs some investigation. Therefore, to unblock the valid changes and enable merging them asap into main, I will remove that line from everywhere, and we can add it in a follow up PR.
@Chemaclass Chemaclass changed the title Strictmode Foundation for strictmode Jul 12, 2024
@Chemaclass Chemaclass marked this pull request as ready for review July 12, 2024 12:32
@Chemaclass
Copy link
Member Author

Chemaclass commented Jul 12, 2024

Adding set -euo pipefail is not possible yet due to some unknown misbehaviours(🐛 ?)
I will rather work in a follow up PR the fix for it, enabling us to merge the changes of this PR.

For example, ./bashunit tests/acceptance/mock_test.sh without debugging mode (set -x):
Screenshot 2024-07-12 at 14 48 38

And with the debug mode enabled we can already see something, but still is not clear how to fix it:
Screenshot 2024-07-12 at 14 51 06

Maybe @djpohly, @cmayo, @staabm might have some idea on this? 🤔

Follow up issue created: #283

@Chemaclass Chemaclass requested a review from cmayo July 12, 2024 12:56
@staabm
Copy link
Contributor

staabm commented Jul 12, 2024

no ideas on my end to this - sorry.

@Chemaclass Chemaclass merged commit a2eb9bd into main Jul 12, 2024
7 checks passed
@Chemaclass Chemaclass deleted the strictmode branch July 12, 2024 13:18
@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
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants