Re: Review Request 58849: GEODE-2840: add a DUnit test to test concurrent deploy

2017-05-01 Thread Kirk Lund

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


Fix it, then Ship it!




Fix it and ship it!


geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConcurrentDeployDUnitTest.java
Lines 74 (patched)


You should null out the static variable as well:
```java
Invoke.invokeInEveryVM(() -> gfsh = null);
```


- Kirk Lund


On April 28, 2017, 6:02 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58849/
> ---
> 
> (Updated April 28, 2017, 6:02 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2840: add a DUnit test to test concurrent deploy
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/DeployedJar.java 
> f96863f66ab2c853567ba6ffdbe3da47c9bcbdb7 
>   geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java 
> a65cd0f550a9cf5a93ae890c094e5f5c6e2908df 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConcurrentDeployDUnitTest.java
>  PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/test/dunit/rules/JarFileRule.java 
> PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  34506c4fa803a1121c29d240ad7d7aa6a55dca99 
> 
> 
> Diff: https://reviews.apache.org/r/58849/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: Review Request 58849: GEODE-2840: add a DUnit test to test concurrent deploy

2017-04-28 Thread Ken Howe

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




geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConcurrentDeployDUnitTest.java
Lines 64-66 (patched)


Is there a particular reason for the spelling of the variables 
'...Invokation' vs. '...Invocation'. Misspellings of words within variable, 
method, etc. names don't add clarity to the code. Exceptions would be where the 
name might conflict with a keyword, which is not the case here.


- Ken Howe


On April 28, 2017, 6:02 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58849/
> ---
> 
> (Updated April 28, 2017, 6:02 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2840: add a DUnit test to test concurrent deploy
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/DeployedJar.java 
> f96863f66ab2c853567ba6ffdbe3da47c9bcbdb7 
>   geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java 
> a65cd0f550a9cf5a93ae890c094e5f5c6e2908df 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConcurrentDeployDUnitTest.java
>  PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/test/dunit/rules/JarFileRule.java 
> PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  34506c4fa803a1121c29d240ad7d7aa6a55dca99 
> 
> 
> Diff: https://reviews.apache.org/r/58849/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Re: Review Request 58849: GEODE-2840: add a DUnit test to test concurrent deploy

2017-04-28 Thread Jared Stewart

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


Ship it!




Ship It!

- Jared Stewart


On April 28, 2017, 6:02 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58849/
> ---
> 
> (Updated April 28, 2017, 6:02 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2840: add a DUnit test to test concurrent deploy
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/DeployedJar.java 
> f96863f66ab2c853567ba6ffdbe3da47c9bcbdb7 
>   geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java 
> a65cd0f550a9cf5a93ae890c094e5f5c6e2908df 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConcurrentDeployDUnitTest.java
>  PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/test/dunit/rules/JarFileRule.java 
> PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  34506c4fa803a1121c29d240ad7d7aa6a55dca99 
> 
> 
> Diff: https://reviews.apache.org/r/58849/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>



Review Request 58849: GEODE-2840: add a DUnit test to test concurrent deploy

2017-04-28 Thread Jinmei Liao

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

Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick 
Rhomberg.


Repository: geode


Description
---

GEODE-2840: add a DUnit test to test concurrent deploy


Diffs
-

  geode-core/src/main/java/org/apache/geode/internal/DeployedJar.java 
f96863f66ab2c853567ba6ffdbe3da47c9bcbdb7 
  geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java 
a65cd0f550a9cf5a93ae890c094e5f5c6e2908df 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConcurrentDeployDUnitTest.java
 PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/JarFileRule.java 
PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 34506c4fa803a1121c29d240ad7d7aa6a55dca99 


Diff: https://reviews.apache.org/r/58849/diff/1/


Testing
---

precheckin running


Thanks,

Jinmei Liao