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

cmd/flux-exec: Fix stdin forwarding bug #2248

Merged
merged 2 commits into from Jul 22, 2019

Conversation

@chu11
Copy link
Contributor

commented Jul 20, 2019

Fix bug in which stdin was not forwarded if the user specified less
than the number of ranks available in the flux instance (i.e. less
than flux_get_size() ranks).

Add new tests appropriately.

Fixes #2247

Copy link
Member

left a comment

Ah I see. Looks reasonable!

Needs a rebase.

Optional: It might be useful to split out the tests to their own commit with a brief description like "cover stdin broadcast when exec runs across a subset of the flux instance".

@chu11 chu11 force-pushed the chu11:issue2247 branch from d4fea05 to 77a1ba5 Jul 22, 2019
@chu11

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

just re-pushed with rebase. Split into two commits and added an extra "test on subset of ranks" test.

chu11 added 2 commits Jul 22, 2019
Fix bug in which stdin was not forwarded if the user specified less
than the number of ranks available in the flux instance (i.e. less
than flux_get_size() ranks).

Fixes #2247
Add tests for subset of ranks using flux-exec.  Includes coverage
for stdin broadcast across a subset of a flux instance for issue #2247.
@chu11 chu11 force-pushed the chu11:issue2247 branch from 77a1ba5 to 4c25342 Jul 22, 2019
@chu11

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

rebased again :-)

@garlick

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

I just set merge-when-passing tag so it should go in once travis finishes.

@mergify mergify bot merged commit 690fe2b into flux-framework:master Jul 22, 2019
2 checks passed
2 checks passed
Summary 1 rule matches
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@codecov-io

This comment has been minimized.

Copy link

commented Jul 22, 2019

Codecov Report

Merging #2248 into master will increase coverage by 0.04%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #2248      +/-   ##
==========================================
+ Coverage   80.72%   80.76%   +0.04%     
==========================================
  Files         210      209       -1     
  Lines       33223    33038     -185     
==========================================
- Hits        26818    26684     -134     
+ Misses       6405     6354      -51
Impacted Files Coverage Δ
src/cmd/flux-exec.c 75.78% <71.42%> (+0.1%) ⬆️
src/common/libflux/message.c 80.62% <0%> (-0.26%) ⬇️
src/cmd/flux-module.c 83.72% <0%> (-0.24%) ⬇️
src/common/libflux/ev_buffer_read.c 93.58% <0%> (-0.09%) ⬇️
src/shell/svc.c
src/shell/task.c 89.77% <0%> (+0.11%) ⬆️
src/common/libsubprocess/remote.c 70.12% <0%> (+0.12%) ⬆️
src/cmd/flux-job.c 85.3% <0%> (+0.3%) ⬆️
src/common/libsubprocess/server.c 71.42% <0%> (+0.56%) ⬆️
src/common/libsubprocess/subprocess.c 88.01% <0%> (+1.15%) ⬆️
... and 3 more
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.