bats/background-process: Close file descriptor 3 #227
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes #226. From the comment within `run_in_background`: Bats duplicates standard output as file descriptor 3 so that output from its framework functions isn't captured along with any output from the code under test. If the code under test contains a `sleep` or other blocking operation, this file descriptor will be held open until the process becomes unblocked, preventing Bats from exiting. Hence, we explicitly close file descriptor 3. Any other code running under Bats that opens a background process should close this file descriptor as well. See: sstephenson/bats#80 Much thanks to @marascio for discovering and researching the problem, and proposing the actual fix.
|
Very strange...Coveralls is starting to count the |
|
The coverage dropped because SimonKagstrom/kcov@0dd22bd added the ability for I'll file a separate PR to resolve this issue before merging this one. I may file a PR against https://github.com/SimonKagstrom/kcov as well, to ensure only files starting with |
mbland
added a commit
that referenced
this pull request
Jun 9, 2018
I noticed during the build for #227 that test coverage dropped precipitously for no apparent reason: https://coveralls.io/builds/14508796 The coverage dropped because SimonKagstrom/kcov@0dd22bd added the ability for kcov to automatically discover all scripts residing in the same directory of the script under test. In this case, it unfortunately discovered all the scripts in ./git and tests/kcov, as well as various *.md and *.yml files that happened to have "bash" in the first line regardless of the presence of a #!. The interim fix for this was to add the --bash-dont-parse-binary-dir and to update the tests accordingly. Also, since a bit of time elapsed since I returned to the problem, I realized the latest tip of kcov's master branch exhibited the hanging bug I helped squash in SimonKagstrom/kcov#211. The bug was reintroduced in SimonKagstrom/kcov@ad17136, which tried to fix a bug reported in SimonKagstrom/kcov#248 whereby kcov would hang on output that didn't contain newlines. SimonKagstrom/kcov#249 attempted to resolve the bug, but didn't quite work and was abandoned. I may take a stab at fixing it one day, but in the meanwhile, kcov v35 with --bash-dont-parse-binary-dir works to get this project's test coverage back in shape.
|
Resolved the coverage issue for now by requiring kcov v35; see #244 for details. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #226. From the comment within
run_in_background:Bats duplicates standard output as file descriptor 3 so that output from its framework functions isn't captured along with any output from the code under test. If the code under test contains a
sleepor other blocking operation, this file descriptor will be held open until the process becomes unblocked, preventing Bats from exiting. Hence, we explicitly close file descriptor 3.Any other code running under Bats that opens a background process should close this file descriptor as well. See: sstephenson/bats#80
Much thanks to @marascio for discovering and researching the problem, and proposing the actual fix.