ESMCI / manage_externals / 181

86%

push

travis-ci

Merge pull request #114 from billsacks/fix_large_output_hang

Fix hangs when a subcommand has a lot of output.
Rework execute_subprocess timeout handling to prevent hangs

The previous implementation, relying on the lower-level
subprocess.Popen, was hanging when trying to get the status of a git
repository with many changed files (see
#113). It seems that the
problem was that subprocess.Popen buffers output, and we would need to
write some extra code to periodically clear the output buffer. This
seemed tricky to get right, so I have backed up to using the
higher-level subprocess.check_output, as we did prior to
#81.

I am using a new method to detect hangs and print a useful message
(which was the point of PR #81), based on threading.Timer. I can't
figure out a way to kill the subprocess when it appears to be hanging,
but at least we can notify the user of this possible problem.

This commit brings this part of the code back to how it was prior to
d6423c6, except now we have a hang-detection timer.

As a side-benefit, it appears that this change speeds up the unit and
system tests, based on a few trial runs before and after this
change. (This is probably due to the removal of the 0.02 sec delay
associated with the previous subprocess.Popen implementation.)

User interface changes?: No

Fixes #113 (Timeout when getting status from a git repository with many changes)

Testing:
test removed: none
unit tests: pass
system tests: pass
manual testing:

(1) Tested the problem reported in #113 by putting a ton of changes in
components/clm (changing permissions on all files, and adding
about 7000 files in a testmods directory). The original hung, this
version succeeded quickly.

(2) Tested the problem reported in #79 and fixed in #81 (because this
PR undoes the changes from #81): I tested with authentication
required (on hobart, by pointing to the NGEET fates repo): I
changed the timeout to 10 seconds, ran
'manage_ex... (continued)

359 of 415 branches covered (86.51%)

Branch coverage included in aggregate %.

1079 of 1163 relevant lines covered (92.78%)

3.7 hits per line