run: don't change the value of i #727

kolyshkin · 2023-05-25T03:23:01Z

I came across an interesting bug today. A very simple test code stopped working properly once I changed "run" followed by the $status check to "run !".

Fixes: #726 (see for a repro).

The fix is to always declare your variables.

I have reviewed the Contributor Guidelines.
I have reviewed the Code of Conduct and agree to abide by it

Instead of using run cmd [ "$status" -ne 0 ] since bats 1.5.0 we can use a simple form: run ! cmd Alas, bats has a bug which makes it change the value of "i" when run is used with any options ([1], [2]). To workaround that, we should not use "i" in any code which uses "run" with options. I know, this is crazy. [1] bats-core/bats-core#726 [2] bats-core/bats-core#727 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

kolyshkin · 2023-05-25T16:34:37Z

Adding a test case is useless, of course, since next time someone will change j instead of i. But I wrote some code to detect such things in general which might be useful; see the #726 (comment)

Instead of using run cmd [ "$status" -ne 0 ] since bats 1.5.0 we can use a simple form: run ! cmd Alas, bats has a bug which makes it change the value of "i" when run is used with any options ([1], [2]). To workaround that, we should not use "i" in any code which uses "run" with options. I know, this is crazy. [1] bats-core/bats-core#726 [2] bats-core/bats-core#727 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

thaJeztah · 2023-05-25T21:08:37Z

One failure in CI (no idea if related in any way);

not ok 324 temp_make() <var>: returns 1 and displays an error message if the directory can not be created
# (in test file bats-file/test/70-temp-10-temp_make.bats, line 32)
#   `[[ ${lines[1]} =~ $REGEX ]] || false' failed
# Last output:
#
# -- ERROR: temp_make --
# mktemp: No such file or directory
# --

martin-schulze-vireso · 2023-05-25T21:58:46Z

~~I am not sure what to make of this. The issue seems to be persistent but I cannot reproduce it locally.~~

It seems the output from mktemp changed (again) in alpine. I fixed the test expecation in bats-core/bats-file@c0f822a

I came across an interesting bug today. A very simple test code stopped working properly once I changed "run" followed by the $status check to "run !". See bats-core#726 for a repro. The fix is to always declare your variables. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

martin-schulze-vireso · 2023-05-25T22:22:46Z

Thanks for your contribution.

Instead of using run cmd [ "$status" -ne 0 ] since bats 1.5.0 we can use a simple form: run ! cmd Alas, bats has a bug which makes it change the value of "i" when run is used with any options ([1], [2]). To workaround that, we should not use "i" in any code which uses "run" with options. I know, this is crazy. [1] bats-core/bats-core#726 [2] bats-core/bats-core#727 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

kolyshkin marked this pull request as ready for review May 25, 2023 03:23

kolyshkin requested a review from a team as a code owner May 25, 2023 03:23

kolyshkin force-pushed the fix-overwrite-i branch from aab64d3 to f648b53 Compare May 25, 2023 03:25

martin-schulze-vireso approved these changes May 25, 2023

View reviewed changes

martin-schulze-vireso force-pushed the fix-overwrite-i branch from f648b53 to 502dc47 Compare May 25, 2023 22:22

martin-schulze-vireso merged commit 3e30d37 into bats-core:master May 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

run: don't change the value of i #727

run: don't change the value of i #727

run: don't change the value of i #727

run: don't change the value of i #727

Conversation