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

testrunner: stream output from subprocesses #680

Merged
merged 2 commits into from Sep 4, 2019
Merged

testrunner: stream output from subprocesses #680

merged 2 commits into from Sep 4, 2019

Conversation

ereslibre
Copy link
Contributor

@ereslibre ereslibre commented Sep 3, 2019

Why is this PR needed?

testrunner: stream output from subprocesses

Fixes: #680

Merge restrictions

(Please do not edit this)

We are in v4-maintenance phase, so we will restrict what can be merged to prevent unexpected surprises:

What can be merged (merge criteria):
    2 approvals:
        1 developer: code is fine
        1 QA: QA is fine
    there is a PR for updating documentation (or a statement that this is not needed)

Copy link
Contributor

@dannysauer dannysauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm gonna go ahead and approve this despite my comment about removing stdbuff. If no one else looks at it and has an opinion, then it's fine to remove it for now and maybe add it back in later if we decide to change the read mechanism to handle those status-generating commands.

@@ -22,6 +22,5 @@ setup_python_env() {
}

setup_python_env
# use unbuffered output
stdbuf -i0 -o0 -e0 python -u ${SDIR}/testrunner.py "$@"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugly as it is, I think we might like to leave the stdbuf commands. There's some desire (SUSE/avant-garde#670) to have tests running within Jenkins or similar generate synchronous output. Imagine something like wget which prints progress output before a newline happens, for example. That, of course, will also require a tweak to how the read_fd threads read from the pipe, but we can deal with that later. This gets us 90% there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to only keep things we really need, otherwise we can just keep certain changes because they were there before without a real use. Honestly, if we need to have a complete unbuffered output let's do that in the future, in my opinion.

Copy link
Contributor

@pablochacin pablochacin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ereslibre ereslibre merged commit fd883b9 into SUSE:master Sep 4, 2019
@ereslibre ereslibre deleted the testrunner-stream-output branch September 4, 2019 10:36
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

3 participants