mbland / go-script-bash

Framework for writing modular, discoverable, testable Bash scripts
ISC License
95 stars 16 forks source link

bats/helpers: Improve create_forwarding_script #195

Closed mbland closed 7 years ago

mbland commented 7 years ago

Also adds create_forwarding_scripts to create multiple forwarding scripts in one call.

The updates to create_forwrading_script addresses several shortcomings that became apparent while using it to test the new test helper skip_if_missing_background_utilities that I'm adding as part of #181.

First, I needed a way to better restrict the PATH of the forwarded command. Trying to set PATH while invoking create_forwarding_script would lead to no forwarding script being created if PATH excluded the directory containing the forwarded program. The new --path option resolves this.

That specific change also resolved a bug where $@ was passed to stub_program_in_path, rather than just the command name argument, causing the script to contain the arguments after the command name.

It then became apparent that the forwarding script needed to exec the wrapped command. As explained in the implementation comments:

The exec feature of the forwarding script is crucial, otherwise the parent process ID within the executed program will be set to the process ID of the wrapper, not the process that invoked the wrapper.

For example, not calling exec confuses bats-exec-test, which invokes itself via bats_perform_tests. As a result, in the parent process, bats_preprocess_source creates BATS_TEST_SOURCE based on BATS_TMPNAME, which is based on $$; then bats_evaluate_preprocessed_source in the child process tries to execute BATS_TEST_SOURCE based on BATS_PARENT_TMPNAME, which is based on $PPID. Since they're different, Bats raises a 'No such file or directory' error.

The correct thing for Bats to do is prefix its self-execution with BATS_TEST_SOURCE="$BATS_TEST_SOURCE", but it's still good practice to use exec here, and to understand why it's important.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 94.648% when pulling e8c629cdb69d51ee48e33767418670e7d0560722 on forwarding-script into e6d646d437f74b68bfa7116b786df4c267d53c12 on master.