Skip to content

[bugfix] Set stdin to null when launching processes from ReFrame, so that processes do no think they are on a tty #1879

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
Mar 29, 2021
Merged

[bugfix] Set stdin to null when launching processes from ReFrame, so that processes do no think they are on a tty #1879

merged 5 commits into from
Mar 29, 2021

Conversation

@rsarm
Copy link
Contributor

@rsarm rsarm commented Mar 23, 2021

Close #1877

@rsarm rsarm added this to the ReFrame sprint 21.03.2 milestone Mar 23, 2021
@rsarm rsarm requested review from ekouts and vkarak March 23, 2021 10:46
@rsarm rsarm self-assigned this Mar 23, 2021
@rsarm rsarm changed the title [bugfix] Fix Popen call to support processes that treat stdin as a terminal [bugfix] Fix Popen call to support processes that treat stdin like a terminal Mar 23, 2021
victorusu
victorusu approved these changes Mar 23, 2021
View reviewed changes
Copy link
Contributor

@victorusu victorusu left a comment

Choose a reason for hiding this comment

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

lgtm

vkarak
vkarak requested changes Mar 23, 2021
View reviewed changes
Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

I would suggest to add a unit test based on the logic of the bug reported in #1877.

vkarak
vkarak requested changes Mar 29, 2021
View reviewed changes
Revert "add unittest"
9254196 
This reverts commit dc7d35f.
@codecov-io
Copy link

codecov-io commented Mar 29, 2021

Codecov Report

Merging #1879 (9254196) into master (13009ac) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1879   +/-   ##
=======================================
  Coverage   87.93%   87.94%           
=======================================
  Files          49       49           
  Lines        8457     8453    -4     
=======================================
- Hits         7437     7434    -3     
+ Misses       1020     1019    -1
Impacted Files Coverage Δ
reframe/utility/osext.py 84.71% <ø> (ø)
reframe/core/meta.py 100.00% <0.00%> (ø)
reframe/core/parameters.py 100.00% <0.00%> (+1.49%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13009ac...9254196. Read the comment docs.

@vkarak vkarak changed the title [bugfix] Fix Popen call to support processes that treat stdin like a terminal [bugfix] Set stdin to null when launching processes from ReFrame, so that process do no think they are on a tty Mar 29, 2021
@vkarak vkarak changed the title [bugfix] Set stdin to null when launching processes from ReFrame, so that process do no think they are on a tty [bugfix] Set stdin to null when launching processes from ReFrame, so that processes do no think they are on a tty Mar 29, 2021
@vkarak vkarak merged commit f890e4d into reframe-hpc:master Mar 29, 2021
@vkarak vkarak mentioned this pull request Mar 18, 2022
Merged
@rsarm rsarm deleted the bugfix/stdin-tty branch February 3, 2023 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vkarak vkarak vkarak approved these changes

@ekouts ekouts ekouts approved these changes

@victorusu victorusu victorusu approved these changes

Assignees

@rsarm rsarm

Labels

bugfix prio: normal

Projects

None yet

Milestone

ReFrame sprint 21.03.2

Development

Successfully merging this pull request may close these issues.

RunOnlyRegressionTest with executables that treat stdin like a tty fail on local scheduler

5 participants

@rsarm @codecov-io @vkarak @ekouts @victorusu