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

Fix a minor bug in the runtests.sh. #216

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

ghost
Copy link

@ghost ghost commented Jul 17, 2023

This commit fixes a minor bug in scripts/runtests.sh: this script is run with POSIX sh on linux machines, meaning that [[ are not recognized. However, [[ … ]] was used (see for instance this CI job).

Because set -e was not set, the minor bug described above did not lead to a CI failure.

This commit fixes the [[ … ]] by using a simple if grep. It also sets set -e at the top of the file in order to catch any potential future mistakes.

Finally, this commit also fixes minor warnings raised by ShellCheck. To display those warnings, one can simply run shellcheck scripts/runtests.sh.

@ghost ghost requested a review from baentsch as a code owner July 17, 2023 15:57
@ghost ghost force-pushed the runtests branch 2 times, most recently from f5ac246 to 21fcdf5 Compare July 17, 2023 16:12
This commit fixes a minor bug in `scripts/runtests.sh`: this script is run with
POSIX sh on linux machines, meaning that `[[` are not recognized.
However, `[[ … ]]` was used (see for instance [this CI job]).

Because `set -e` was not set, the minor bug described above did not lead to a
CI failure.

This commit fixes the `[[ … ]]` by using a simple `if grep`.
It also sets `set -e` at the top of the file in order to catch any potential future mistakes.

Finally, this commit also fixes minor warnings raised by [ShellCheck]. To display those warnings,
one can simply run `shellcheck scripts/runtests.sh`.

[this CI job]: https://github.com/open-quantum-safe/oqs-provider/actions/runs/5527705776/jobs/10083733262#step:6:14
[ShellCheck]: https://www.shellcheck.net/
@ghost ghost force-pushed the runtests branch from 21fcdf5 to f9e4b77 Compare July 17, 2023 16:24
Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

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

Thanks for making this more robust and correct!

@baentsch baentsch merged commit 5250576 into open-quantum-safe:main Jul 18, 2023
@ghost ghost deleted the runtests branch July 18, 2023 06:16
feventura pushed a commit to EntrustCorporation/oqs-provider that referenced this pull request Mar 16, 2024
This commit fixes a minor bug in `scripts/runtests.sh`: this script is run with
POSIX sh on linux machines, meaning that `[[` are not recognized.
However, `[[ … ]]` was used.

Because `set -e` was not set, the minor bug described above did not lead to a
CI failure.

This commit fixes the `[[ … ]]` by using a simple `if grep`.
It also sets `set -e` at the top of the file in order to catch any potential future mistakes.

Finally, this commit also fixes minor warnings raised by [ShellCheck]. To display those warnings,
one can simply run `shellcheck scripts/runtests.sh`.

[ShellCheck]: https://www.shellcheck.net/

Signed-off-by: Felipe Ventura <felipe.ventura@entrust.com>
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.

1 participant