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

Remove broken agrep test entry (fails with bash >= 5.2) #87

Closed
wants to merge 1 commit into from

Conversation

Vogtinator
Copy link

@Vogtinator Vogtinator commented Oct 11, 2022

It's meant to cause agrep to return with exit code 2, but asserts that it's exit code 1 instead.

It's meant to ensure that using .* as pattern results in exit code 2 because it matches also an empty string. However, glob expansion results in .* picking up files such as . and .. from the CWD, which get interpreted as valid pattern. This results in exit status 1 (no match found) which is what the .ok file expects, but that's invalid.

With bash 5.2, glob expansion no longer matches . and .. by default, so the test works as intended by accident, causing a mismatch with the expected wrong exit code.

It's unfortunately not easily possible to avoid glob expansion in this case.

Just remove the test for now.

It's meant to cause agrep to return with exit code 2, but asserts that it's
exit code 1 instead.

It's meant to ensure that using ".*" as pattern results in exit code 2 because
it matches also an empty string. However, glob expansion results in ".*"
picking up files such as "." and ".." from the CWD, which get interpreted as
valid pattern. This results in exit status 1 (no match found) which is what
the .ok file expects, but that's invalid.

With bash 5.2, glob expansion no longer matches "." and ".." by default, so
the test works as intended by accident, causing a mismatch with the expected
wrong exit code.

It's unfortunately not easily possible to avoid glob expansion in this case.

Just remove the test for now.
@trushworth
Copy link
Collaborator

Bash 4.4 (and up) and sh seem to have a "noglob" option to disable shell globbing. Would it make sense to add
set -f
and
set +f
to tests/agrep/run-tests.sh around the line that ends up doing the unwanted globbing? I'll see if I can set up something to test the before and after cases. If it solves the problem then exitstatus.ok will need to be updated too.

@trushworth
Copy link
Collaborator

I tried the 'set -f' idea and it worked just fine in FreeBSD, Linux, and MacOS, so I've gone with that approach. Thank you very much for the diagnosis though, the problem would have been much much more work to figure out without it!

@trushworth trushworth closed this Jun 2, 2023
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.

None yet

2 participants