Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Bigtop >> mail # dev >> Review Request: BIGTOP-835. The shell exec method must have variants which have timeout and can run in background


Copy link to this message
-
Re: Review Request: BIGTOP-835. The shell exec method must have variants which have timeout and can run in background

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9166/#review15917
-----------------------------------------------------------

bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
<https://reviews.apache.org/r/9166/#comment34197>

    NItpick: If all you want is an empty list, could you use Collections.EMPTY_LIST?

bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
<https://reviews.apache.org/r/9166/#comment34198>

    If you do decide to change out to be Collections.EMPTY_LIST, could you please change this to be the same as well for consistency?

bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy
<https://reviews.apache.org/r/9166/#comment34201>

    Wouldn't a better test to be have something that never finishes (like an infinite loop) and have a timeout on that? Test would succeed asserting if it finished (possibly within a certain time).

bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy
<https://reviews.apache.org/r/9166/#comment34199>

    Why 4000? Could you please add a comment stating so?

bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy
<https://reviews.apache.org/r/9166/#comment34200>

    Same here
- Mark Grover
On Jan. 31, 2013, 1:37 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9166/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2013, 1:37 a.m.)
>
>
> Review request for bigtop and Roman Shaposhnik.
>
>
> Description
> -------
>
> Added new methods which can execute a script for a specified timeout and can also execute in the background.
>
>
> This addresses bug BIGTOP-835.
>     https://issues.apache.org/jira/browse/BIGTOP-835
>
>
> Diffs
> -----
>
>   bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy ae3da68
>   bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy 1571e10
>
> Diff: https://reviews.apache.org/r/9166/diff/
>
>
> Testing
> -------
>
> Added unit tests for new functionality. Current unit tests pass
>
>
> Thanks,
>
> Hari Shreedharan
>
>