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

Tests fail with bats 1.5.0 #121

Closed
rycee opened this issue Nov 1, 2021 · 7 comments
Closed

Tests fail with bats 1.5.0 #121

rycee opened this issue Nov 1, 2021 · 7 comments

Comments

@rycee
Copy link

rycee commented Nov 1, 2021

See below for a test run with Bats 1.5.0 (the same run succeeds with Bats 1.4.1). Originally found in a NixOS CI job. This is with bash-preexec 0.4.1 but master fails in the same way.

I'm not completely certain what the cause is but the line

Error: Test file "/home/rycee/devel/nixpkgs/source/merge" does not exist

seems problematic. I wonder if not the "merge" in the error above comes from the "merge" in bats_merge_stdout_and_stderr?

$ bats test
 ✓ __bp_install_after_session_init should exit with 1 if we're not using bash
 ✓ __bp_install should exit if it's already installed
 ✓ __bp_install should remove trap logic and itself from PROMPT_COMMAND
 ✓ __bp_install should preserve an existing DEBUG trap
 ✓ __bp_sanitize_string should remove semicolons and trim space
 ✓ Appending to PROMPT_COMMAND should work after bp_install
 ✓ Appending or prepending to PROMPT_COMMAND should work after bp_install_after_session_init
 ✓ Adding to PROMPT_COMMAND before and after initiating install
 ✓ Adding to PROMPT_COMMAND after with semicolon
 ✓ during install PROMPT_COMMAND and precmd functions should be executed each once
 ✓ No functions defined for preexec should simply return
 ✓ precmd should execute a function once
 ✓ precmd should set $? to be the previous exit code
 ✓ precmd should set $BP_PIPESTATUS to the previous $PIPESTATUS
 ✓ precmd should set $_ to be the previous last arg
 ✓ preexec should execute a function with the last command in our history
 ✓ preexec should execute multiple functions in the order added to their arrays
 ✓ preecmd should execute multiple functions in the order added to their arrays
 ✗ preexec should execute a function with IFS defined to local scope
   (in test file test/bash-preexec.bats, line 260)
     `[ $status -eq 0 ]' failed
   Error: Test file "/home/rycee/devel/nixpkgs/source/merge" does not exist
 ✗ precmd should execute a function with IFS defined to local scope
   (in test file test/bash-preexec.bats, line 270)
     `[ $status -eq 0 ]' failed
   Error: Test file "/home/rycee/devel/nixpkgs/source/merge" does not exist
 ✓ preexec should set $? to be the exit code of preexec_functions
 ✓ in_prompt_command should detect if a command is part of PROMPT_COMMAND
 ✓ __bp_adjust_histcontrol should remove ignorespace and ignoreboth
 ✓ preexec should respect HISTTIMEFORMAT
 ✓ preexec should not strip whitespace from commands
 ✓ preexec should preserve multi-line strings in commands
 ✓ preexec should work on options to 'echo' commands
 ✓ should not import if it's already defined
 ✓ should import if not defined
 ✓ bp should stop installation if HISTTIMEFORMAT is readonly

30 tests, 2 failures
@benjamb
Copy link

benjamb commented Nov 16, 2022

@rycee This should be fixed in bats 1.8.0. The relevant fix is: bats-core/bats-core@c8b5bba

@akinomyoga
Copy link
Contributor

Ah, this is reported here. I actually submitted the fix to bats-core for the test failure in bash-preexec but haven't recognized that the issue was already reported here. Thank you for the comment. @rcaloras Now I think this issue can be closed.

(And, now I noticed my typo in the commit message s/sepcial/special/...)

rcaloras added a commit that referenced this issue Nov 16, 2022
- Related to #121. Didn't notice this was failing until upgrading to
  bats 1.8.2 and the related IFS issue was fixed.
@rcaloras
Copy link
Owner

Great. Thanks as always @akinomyoga!

@rycee
Copy link
Author

rycee commented Nov 17, 2022

Thanks, I tester just now with bats 1.8.2 and curiously I get

ok 19 preexec should execute a function with IFS defined to local scope
not ok 20 precmd should execute a function with IFS defined to local scope
# (in test file test/bash-preexec.bats, line 272)
#   `[ $status -eq 0 ]' failed
ok 21 preexec should set $? to be the exit code of preexec_function

@akinomyoga
Copy link
Contributor

akinomyoga commented Nov 17, 2022

I'm not sure why I overlooked this, but this seems to be an oversight of updating tests in #131. Lines 212 and 248 in test/bash-preexec.bats seem to have been correctly updated, but line 280 was not updated at that time. I guess this is because this test failure had been hidden by the bug of bats-core, but it is revealed now that bats-core is fixed by bats-core/bats-core#650.

akinomyoga added a commit to akinomyoga/bash-preexec that referenced this issue Nov 17, 2022
This fixes an issue reported by @rycee (Robert Helgesson) [1].

[1] rcaloras#121 (comment)
@akinomyoga
Copy link
Contributor

@rycee I realized that this is actually already fixed in the master by @rcaloras 17 hours ago! You can pull the latest master and try it!

@rycee
Copy link
Author

rycee commented Nov 17, 2022

@akinomyoga Indeed, with latest master all tests pass. Thanks!

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

No branches or pull requests

4 participants