Skip to content

Fix a problem that run always fails with IFS=_ #650

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Sep 14, 2022
Merged

Fix a problem that run always fails with IFS=_ #650

merged 5 commits into from
Sep 14, 2022

Conversation

akinomyoga
Copy link
Contributor

Environment and steps to reproduce the issue

Bugs/usage issues

  1. State the version of Bash you're using bash --version
$ LANG=C bash --version
GNU bash, version 5.1.16(1)-release (x86_64-redhat-linux-gnu)
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
  1. State your operating system and its version
$ cat /etc/os-release
NAME="Fedora Linux"
VERSION="36 (Server Edition)"
ID=fedora
VERSION_ID=36
VERSION_CODENAME=""
PLATFORM_ID="platform:f36"
PRETTY_NAME="Fedora Linux 36 (Server Edition)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:36"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f36/system-administrators-guide/"
SUPPORT_URL="https://ask.fedoraproject.org/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=36
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=36
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
VARIANT="Server Edition"
VARIANT_ID=server
  1. Command line steps or code snippets that reproduce the issue
$ bats --version
Bats 1.7.0
$ cat test1.bats
@test "test something with a special IFS" {
  IFS=_
  run true
  [ "$status" -eq 0 ]
}
$ bats test1.bats
test1.bats
 ✗ test something with a special IFS
   (in test file test1.bats, line 4)
     `[ "$status" -eq 0 ]' failed
   Error: Test file "/home/murase/.mwg/git/bats-core/bats-core/t/merge" does not exist

1 test, 1 failure

$
  1. Any apparently relevant information from the Bash changelog

I believe this is unrelated to any changes in Bash.

Issues and fixes

I wanted to perform a test with a special value of IFS but noticed that run always fails with IFS=_. This is caused by unquoted $pre_command in this line. The variable pre_command contains a function name including _, so the function name is split by IFS=_ by word splitting. To fix the issue, we can just quote the function name by " ... ". See c8b5bba.

I also noticed that run tries to recover the original value of IFS, but this fails to restore the original state of IFS when IFS is in the unset state before the test. It should be noted that the empty IFS and the unset IFS have different meanings: e.g. word splitting is disabled by the empty IFS while word splitting is performed with the unset IFS as if it has the default value $' \t\n'. To fix the issue, we may just declare IFS locally in the function. We may also save and restore the set/unset state of IFS as well as its value in the function. See d62e928.

@akinomyoga akinomyoga requested a review from a team as a code owner September 11, 2022 19:56
@akinomyoga akinomyoga mentioned this pull request Sep 11, 2022
Merged
2 tasks
@akinomyoga akinomyoga mentioned this pull request Sep 12, 2022
Closed
2 tasks
@martin-schulze-vireso martin-schulze-vireso merged commit a226e46 into bats-core:master Sep 14, 2022
@martin-schulze-vireso
Copy link
Member

Thanks for you contribution. I took the liberty to reassess the code and decided that preserving IFS is not needed anymore, due to earlier code changes.

@akinomyoga akinomyoga deleted the IFS branch September 14, 2022 22:00
@akinomyoga
Copy link
Contributor Author

Thank you!

@akinomyoga akinomyoga mentioned this pull request Nov 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@akinomyoga @martin-schulze-vireso