Re: Review Request 58325: GEODE-2737: Change Pulse UI tests to use random port for JMX connections

[Jinmei Liao]

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


Ship it!




Ship It!

- Jinmei Liao


On April 12, 2017, 4:02 p.m., Ken Howe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58325/
> ---
> 
> (Updated April 12, 2017, 4:02 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2737: Change Pulse UI tests to use random port for JMX connections
> 
> 
> Diffs
> -
> 
>   geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 
> ebf291b481c78533adab9006e32a9dcb5d1d8d39 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
>  af3d64bd5118ff870fadc16dcdeb3cde858611ce 
> 
> 
> Diff: https://reviews.apache.org/r/58325/diff/3/
> 
> 
> Testing
> ---
> 
> ==> Latest precheckin was clean.
> 
> All Pulse UI tests run locally pass.
> 
> Precheckin is running
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Re: Review Request 58325: GEODE-2737: Change Pulse UI tests to use random port for JMX connections

[Ken Howe]

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

(Updated April 12, 2017, 4:02 p.m.)


Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
Rhomberg.


Repository: geode


Description
---

GEODE-2737: Change Pulse UI tests to use random port for JMX connections


Diffs
-

  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 
ebf291b481c78533adab9006e32a9dcb5d1d8d39 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
 af3d64bd5118ff870fadc16dcdeb3cde858611ce 


Diff: https://reviews.apache.org/r/58325/diff/3/


Testing (updated)
---

==> Latest precheckin was clean.

All Pulse UI tests run locally pass.

Precheckin is running


Thanks,

Ken Howe

Re: Review Request 58325: GEODE-2737: Change Pulse UI tests to use random port for JMX connections

[Ken Howe]

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

(Updated April 11, 2017, 9:28 p.m.)


Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
Rhomberg.


Changes
---

Ran spotlessApply to reformat the previous change


Repository: geode


Description
---

GEODE-2737: Change Pulse UI tests to use random port for JMX connections


Diffs (updated)
-

  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 
ebf291b481c78533adab9006e32a9dcb5d1d8d39 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
 af3d64bd5118ff870fadc16dcdeb3cde858611ce 


Diff: https://reviews.apache.org/r/58325/diff/3/

Changes: https://reviews.apache.org/r/58325/diff/2-3/


Testing (updated)
---

==> Precheckin was clean other than spotlessCheck. Re-runing the precheckin 
after reformatting to correct the failure

All Pulse UI tests run locally pass.

Precheckin is running


Thanks,

Ken Howe

Re: Review Request 58325: GEODE-2737: Change Pulse UI tests to use random port for JMX connections

[Ken Howe]

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

(Updated April 11, 2017, 5:18 p.m.)


Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
Rhomberg.


Changes
---

Inn reponse to Kirk's comment, I renamed parameter port to jmxPort in 
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java to 
clarify it's usage.


Repository: geode


Description
---

GEODE-2737: Change Pulse UI tests to use random port for JMX connections


Diffs (updated)
-

  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 
ebf291b481c78533adab9006e32a9dcb5d1d8d39 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
 af3d64bd5118ff870fadc16dcdeb3cde858611ce 


Diff: https://reviews.apache.org/r/58325/diff/2/

Changes: https://reviews.apache.org/r/58325/diff/1-2/


Testing (updated)
---

==> Concourse had a problem on the prechecking while running flaky tests
==> precheckin has been restarted

All Pulse UI tests run locally pass.

Precheckin is running


Thanks,

Ken Howe

Re: Review Request 58325: GEODE-2737: Change Pulse UI tests to use random port for JMX connections

[Jinmei Liao]

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


Ship it!




Ship It!

- Jinmei Liao


On April 10, 2017, 9:52 p.m., Ken Howe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58325/
> ---
> 
> (Updated April 10, 2017, 9:52 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2737: Change Pulse UI tests to use random port for JMX connections
> 
> 
> Diffs
> -
> 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
>  af3d64bd5118ff870fadc16dcdeb3cde858611ce 
> 
> 
> Diff: https://reviews.apache.org/r/58325/diff/1/
> 
> 
> Testing
> ---
> 
> All Pulse UI tests run locally pass.
> 
> Precheckin is running
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Re: Review Request 58325: GEODE-2737: Change Pulse UI tests to use random port for JMX connections

[Jinmei Liao]

> On April 10, 2017, 10:05 p.m., Kirk Lund wrote:
> > geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
> > Lines 47 (patched)
> > 
> >
> > Is there no other way to configure this other than with hidden system 
> > properties? I was hoping there would be something in 
> > ConfigurationProperties to configure the Pulse port. I suppose that 
> > LauncherLifecycleCommands#startPulse would already be configuring things 
> > but it's rather sparse for configuration.
> > 
> > Also, I notice that you're using "jmxPort" for the Pulse port. 
> > Shouldn't this be "pulsePort"? The "jmxPort" would theoretically be the RMI 
> > server port for JMX instead.
> 
> Kirk Lund wrote:
> I'm guess that "pulse.port" is actually the jmxPort now that I'm reading 
> PulseAppListener. Go ahead and disregard the jmxPort vs pulsePort question. 
> Confusing code in Pulse...
> 
> Jared Stewart wrote:
> One thing to note is that `start pulse` only opens a web browser to the 
> pulse URL.  It doesn't actually start pulse AFAIK. (In the default 
> configuration, pulse will start up when you issue `start locator`.)

When Kevin was fixing the "pulse always tries to connect to 1099" issues, used 
this hidden system properties to tell pulse what jmx port locator is running 
on, since Pulse can not used any class reference from geode-core.


- Jinmei


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


On April 10, 2017, 9:52 p.m., Ken Howe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58325/
> ---
> 
> (Updated April 10, 2017, 9:52 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2737: Change Pulse UI tests to use random port for JMX connections
> 
> 
> Diffs
> -
> 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
>  af3d64bd5118ff870fadc16dcdeb3cde858611ce 
> 
> 
> Diff: https://reviews.apache.org/r/58325/diff/1/
> 
> 
> Testing
> ---
> 
> All Pulse UI tests run locally pass.
> 
> Precheckin is running
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Re: Review Request 58325: GEODE-2737: Change Pulse UI tests to use random port for JMX connections

[Jared Stewart]

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


Ship it!




Ship It!

- Jared Stewart


On April 10, 2017, 9:52 p.m., Ken Howe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58325/
> ---
> 
> (Updated April 10, 2017, 9:52 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2737: Change Pulse UI tests to use random port for JMX connections
> 
> 
> Diffs
> -
> 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
>  af3d64bd5118ff870fadc16dcdeb3cde858611ce 
> 
> 
> Diff: https://reviews.apache.org/r/58325/diff/1/
> 
> 
> Testing
> ---
> 
> All Pulse UI tests run locally pass.
> 
> Precheckin is running
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Re: Review Request 58325: GEODE-2737: Change Pulse UI tests to use random port for JMX connections

[Kirk Lund]

> On April 10, 2017, 10:05 p.m., Kirk Lund wrote:
> > geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
> > Lines 47 (patched)
> > 
> >
> > Is there no other way to configure this other than with hidden system 
> > properties? I was hoping there would be something in 
> > ConfigurationProperties to configure the Pulse port. I suppose that 
> > LauncherLifecycleCommands#startPulse would already be configuring things 
> > but it's rather sparse for configuration.
> > 
> > Also, I notice that you're using "jmxPort" for the Pulse port. 
> > Shouldn't this be "pulsePort"? The "jmxPort" would theoretically be the RMI 
> > server port for JMX instead.

I'm guess that "pulse.port" is actually the jmxPort now that I'm reading 
PulseAppListener. Go ahead and disregard the jmxPort vs pulsePort question. 
Confusing code in Pulse...


- Kirk


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


On April 10, 2017, 9:52 p.m., Ken Howe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58325/
> ---
> 
> (Updated April 10, 2017, 9:52 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2737: Change Pulse UI tests to use random port for JMX connections
> 
> 
> Diffs
> -
> 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
>  af3d64bd5118ff870fadc16dcdeb3cde858611ce 
> 
> 
> Diff: https://reviews.apache.org/r/58325/diff/1/
> 
> 
> Testing
> ---
> 
> All Pulse UI tests run locally pass.
> 
> Precheckin is running
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Re: Review Request 58325: GEODE-2737: Change Pulse UI tests to use random port for JMX connections

[Kirk Lund]

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




geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
Lines 47 (patched)


Is there no other way to configure this other than with hidden system 
properties? I was hoping there would be something in ConfigurationProperties to 
configure the Pulse port. I suppose that LauncherLifecycleCommands#startPulse 
would already be configuring things but it's rather sparse for configuration.

Also, I notice that you're using "jmxPort" for the Pulse port. Shouldn't 
this be "pulsePort"? The "jmxPort" would theoretically be the RMI server port 
for JMX instead.


- Kirk Lund


On April 10, 2017, 9:52 p.m., Ken Howe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58325/
> ---
> 
> (Updated April 10, 2017, 9:52 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2737: Change Pulse UI tests to use random port for JMX connections
> 
> 
> Diffs
> -
> 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
>  af3d64bd5118ff870fadc16dcdeb3cde858611ce 
> 
> 
> Diff: https://reviews.apache.org/r/58325/diff/1/
> 
> 
> Testing
> ---
> 
> All Pulse UI tests run locally pass.
> 
> Precheckin is running
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Review Request 58325: GEODE-2737: Change Pulse UI tests to use random port for JMX connections

[Ken Howe]

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

Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
Rhomberg.


Repository: geode


Description
---

GEODE-2737: Change Pulse UI tests to use random port for JMX connections


Diffs
-

  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
 af3d64bd5118ff870fadc16dcdeb3cde858611ce 


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


Testing
---

All Pulse UI tests run locally pass.

Precheckin is running


Thanks,

Ken Howe