You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by John Sirois <js...@apache.org> on 2016/01/09 04:27:55 UTC

Review Request 42102: Fix flaky `ServerSetImplTest` test.

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

Review request for Aurora, George Sirois and Bill Farner.


Bugs: AURORA-1574
    https://issues.apache.org/jira/browse/AURORA-1574


Repository: aurora


Description
-------

The `testUnwatchOnException` test method uses a forced
InterruptedException to test that watches are un-registered.  In doing
so, the test inadvertantly set the interrupt bit for the test runner
thread, poisoning subsequent tests that invoked blocking code.  The
poising was only evident when the test methods were not run in lexical
order, which is the case in the vagrant vm.  This fix explicitly clears
the interrupt bit for the test thread with an explanation of why this is
done.

 commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)


Diffs
-----

  commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java d446a99ccaaecea01ca8d59f72378b2e6f60bbc3 

Diff: https://reviews.apache.org/r/42102/diff/


Testing
-------

Before the fix, this consistent error in the vagrant VM:
```
vagrant@aurora:~/aurora$ ./gradlew --rerun-tasks commons:test --tests org.apache.aurora.common.zookeeper.ServerSetImplTest
...
org.apache.aurora.common.zookeeper.ServerSetImplTest > testOrdering FAILED
    org.apache.aurora.common.net.pool.DynamicHostSet$MonitorException at ServerSetImplTest.java:155
        Caused by: org.apache.aurora.common.zookeeper.Group$WatchException at ServerSetImplTest.java:155
            Caused by: org.apache.aurora.common.zookeeper.Group$JoinException at ServerSetImplTest.java:155
                Caused by: java.lang.InterruptedException at ServerSetImplTest.java:155
```

Green after the fix in the vm and when run normally on my machine.


Thanks,

John Sirois


Re: Review Request 42102: Fix flaky `ServerSetImplTest` test.

Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42102/
-----------------------------------------------------------

(Updated Jan. 9, 2016, 11:09 a.m.)


Review request for Aurora, George Sirois and Bill Farner.


Bugs: AURORA-1574
    https://issues.apache.org/jira/browse/AURORA-1574


Repository: aurora


Description (updated)
-------

The `testUnwatchOnException` test method uses a forced
InterruptedException to test that watches are un-registered.  In doing
so, the test inadvertantly set the interrupt bit for the test runner
thread, poisoning subsequent tests that invoked blocking code.  The
poisoning was only evident when the test methods were not run in lexical
order, which is the case in the vagrant vm.  This fix explicitly clears
the interrupt bit for the test thread with an explanation of why this is
done.

 commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)


Diffs
-----

  commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java d446a99ccaaecea01ca8d59f72378b2e6f60bbc3 

Diff: https://reviews.apache.org/r/42102/diff/


Testing
-------

Before the fix, this consistent error in the vagrant VM:
```
vagrant@aurora:~/aurora$ ./gradlew --rerun-tasks commons:test --tests org.apache.aurora.common.zookeeper.ServerSetImplTest
...
org.apache.aurora.common.zookeeper.ServerSetImplTest > testOrdering FAILED
    org.apache.aurora.common.net.pool.DynamicHostSet$MonitorException at ServerSetImplTest.java:155
        Caused by: org.apache.aurora.common.zookeeper.Group$WatchException at ServerSetImplTest.java:155
            Caused by: org.apache.aurora.common.zookeeper.Group$JoinException at ServerSetImplTest.java:155
                Caused by: java.lang.InterruptedException at ServerSetImplTest.java:155
```

Green after the fix in the vm and when run normally on my machine.


Thanks,

John Sirois


Re: Review Request 42102: Fix flaky `ServerSetImplTest` test.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42102/#review113602
-----------------------------------------------------------

Ship it!


Master (b25ab87) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On Jan. 9, 2016, 3:27 a.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42102/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2016, 3:27 a.m.)
> 
> 
> Review request for Aurora, George Sirois and Bill Farner.
> 
> 
> Bugs: AURORA-1574
>     https://issues.apache.org/jira/browse/AURORA-1574
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The `testUnwatchOnException` test method uses a forced
> InterruptedException to test that watches are un-registered.  In doing
> so, the test inadvertantly set the interrupt bit for the test runner
> thread, poisoning subsequent tests that invoked blocking code.  The
> poising was only evident when the test methods were not run in lexical
> order, which is the case in the vagrant vm.  This fix explicitly clears
> the interrupt bit for the test thread with an explanation of why this is
> done.
> 
>  commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> 
> Diffs
> -----
> 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java d446a99ccaaecea01ca8d59f72378b2e6f60bbc3 
> 
> Diff: https://reviews.apache.org/r/42102/diff/
> 
> 
> Testing
> -------
> 
> Before the fix, this consistent error in the vagrant VM:
> ```
> vagrant@aurora:~/aurora$ ./gradlew --rerun-tasks commons:test --tests org.apache.aurora.common.zookeeper.ServerSetImplTest
> ...
> org.apache.aurora.common.zookeeper.ServerSetImplTest > testOrdering FAILED
>     org.apache.aurora.common.net.pool.DynamicHostSet$MonitorException at ServerSetImplTest.java:155
>         Caused by: org.apache.aurora.common.zookeeper.Group$WatchException at ServerSetImplTest.java:155
>             Caused by: org.apache.aurora.common.zookeeper.Group$JoinException at ServerSetImplTest.java:155
>                 Caused by: java.lang.InterruptedException at ServerSetImplTest.java:155
> ```
> 
> Green after the fix in the vm and when run normally on my machine.
> 
> 
> Thanks,
> 
> John Sirois
> 
>


Re: Review Request 42102: Fix flaky `ServerSetImplTest` test.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42102/#review113606
-----------------------------------------------------------

Ship it!


Nice sleuthing!

- Bill Farner


On Jan. 8, 2016, 7:27 p.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42102/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2016, 7:27 p.m.)
> 
> 
> Review request for Aurora, George Sirois and Bill Farner.
> 
> 
> Bugs: AURORA-1574
>     https://issues.apache.org/jira/browse/AURORA-1574
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The `testUnwatchOnException` test method uses a forced
> InterruptedException to test that watches are un-registered.  In doing
> so, the test inadvertantly set the interrupt bit for the test runner
> thread, poisoning subsequent tests that invoked blocking code.  The
> poising was only evident when the test methods were not run in lexical
> order, which is the case in the vagrant vm.  This fix explicitly clears
> the interrupt bit for the test thread with an explanation of why this is
> done.
> 
>  commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> 
> Diffs
> -----
> 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java d446a99ccaaecea01ca8d59f72378b2e6f60bbc3 
> 
> Diff: https://reviews.apache.org/r/42102/diff/
> 
> 
> Testing
> -------
> 
> Before the fix, this consistent error in the vagrant VM:
> ```
> vagrant@aurora:~/aurora$ ./gradlew --rerun-tasks commons:test --tests org.apache.aurora.common.zookeeper.ServerSetImplTest
> ...
> org.apache.aurora.common.zookeeper.ServerSetImplTest > testOrdering FAILED
>     org.apache.aurora.common.net.pool.DynamicHostSet$MonitorException at ServerSetImplTest.java:155
>         Caused by: org.apache.aurora.common.zookeeper.Group$WatchException at ServerSetImplTest.java:155
>             Caused by: org.apache.aurora.common.zookeeper.Group$JoinException at ServerSetImplTest.java:155
>                 Caused by: java.lang.InterruptedException at ServerSetImplTest.java:155
> ```
> 
> Green after the fix in the vm and when run normally on my machine.
> 
> 
> Thanks,
> 
> John Sirois
> 
>