IdlingResource for destroy() VideoFileRenderer

Espresso test for loopback video quality testing

Using IdlingResource in CallActivity to tell Espresso it is busy during shutdown.

BUG=webrtc:7185

[mandermo, 2017-03-09 15:36:29]

Description was changed from

==========
IdlingResource for destroy() VideoFileRenderer

BUG=
==========

to

==========
Espresso test for loopback video quality testing

Using IdlingResource in CallActivity to tell Espresso it is busy during
shutdown.

BUG=7185
==========

[mandermo, 2017-03-09 15:36:29]

mandermo@webrtc.org changed reviewers:
+ kjellander@webrtc.org

[mandermo, 2017-03-09 15:37:17]

Description was changed from

==========
Espresso test for loopback video quality testing

Using IdlingResource in CallActivity to tell Espresso it is busy during
shutdown.

BUG=7185
==========

to

==========
Espresso test for loopback video quality testing

Using IdlingResource in CallActivity to tell Espresso it is busy during
shutdown.

BUG=webrtc:7185
==========

[kjellander_webrtc, 2017-03-10 10:40:38]

https://codereview.webrtc.org/2741743002/diff/20001/webrtc/examples/androidte...
File
webrtc/examples/androidtests/src/org/appspot/apprtc/test/ConnectActivityStubbedInputOutputTest.java
(right):

https://codereview.webrtc.org/2741743002/diff/20001/webrtc/examples/androidte...
webrtc/examples/androidtests/src/org/appspot/apprtc/test/ConnectActivityStubbedInputOutputTest.java:58:
// context, IdlingResourceCallActivity.class);
Please remove commented code.

https://codereview.webrtc.org/2741743002/diff/20001/webrtc/examples/androidte...
webrtc/examples/androidtests/src/org/appspot/apprtc/test/ConnectActivityStubbedInputOutputTest.java:126:
// onView(withId(R.id.button_call_disconnect)).perform(click());
Should this be restored?

[mandermo, 2017-03-10 12:07:43]

Rename of ConnectActivityStubbedInputOutputTest and removal of commented code
and comment about commented code.

[kjellander_webrtc, 2017-03-10 12:14:13]

https://codereview.webrtc.org/2741743002/diff/40001/webrtc/examples/androidte...
File
webrtc/examples/androidtests/src/org/appspot/apprtc/test/CallActivityStubbedInputOutputTest.java
(right):

https://codereview.webrtc.org/2741743002/diff/40001/webrtc/examples/androidte...
webrtc/examples/androidtests/src/org/appspot/apprtc/test/CallActivityStubbedInputOutputTest.java:119:
// XXX: Not needed at the moment to finish the call.
What does this mean "not at the moment"? When will it be needed?
If it's not needed ever again, remove all of 116-121, or why would we need the
sleep?

[kjellander_webrtc, 2017-03-17 12:02:16]

I'm afraid the trybot won't be of any use here - it just runs the
out/Release/bin/run_AppRTCMobileTestStubbedVideoIO which will fail because the
stuff in video_quality_loopback_test.py that brings up AppRTC hasn't executed.

We only have two options to try this out: submit and pray for the best or
ordering a new machine with a dedicated device for the try server. The latter
will take a long time, so if this passes a local run on a device with WIFI
disabled, I think it's worth a try to submit and try again to enable it on the
bots.

https://codereview.webrtc.org/2741743002/diff/140001/webrtc/examples/androidt...
File webrtc/examples/androidtests/video_quality_loopback_test.py (right):

https://codereview.webrtc.org/2741743002/diff/140001/webrtc/examples/androidt...
webrtc/examples/androidtests/video_quality_loopback_test.py:83: dev_appserver =
os.path.join(SRC_DIR, 'out', 'apprtc', 'google_appengine',
Add try+finally around the processes being launched so you ensure you terminate
them no matter what happens.

I guess you have two options here, defining all three variables (apprtc_process,
collider_process, reverseforwarder_process) to none outside a single try+finally
clause, or nest three different ones inside each other. I think the former is
less cluttered.

I think you also should implement a SIGINT handler to protect against zombie
processes staying around if ctrl+c is pressed during execution (or if the test
is interrupted when running on the bot).
See http://stackoverflow.com/questions/1112343/how-do-i-capture-sigint-in-python
for example.

[mandermo, 2017-03-17 13:39:42]

https://codereview.webrtc.org/2741743002/diff/140001/webrtc/examples/androidt...
File webrtc/examples/androidtests/video_quality_loopback_test.py (right):

https://codereview.webrtc.org/2741743002/diff/140001/webrtc/examples/androidt...
webrtc/examples/androidtests/video_quality_loopback_test.py:83: dev_appserver =
os.path.join(SRC_DIR, 'out', 'apprtc', 'google_appengine',
On 2017/03/17 12:02:16, kjellander_webrtc wrote:
> Add try+finally around the processes being launched so you ensure you
terminate
> them no matter what happens.
> 
> I guess you have two options here, defining all three variables
(apprtc_process,
> collider_process, reverseforwarder_process) to none outside a single
try+finally
> clause, or nest three different ones inside each other. I think the former is
> less cluttered.
> 
> I think you also should implement a SIGINT handler to protect against zombie
> processes staying around if ctrl+c is pressed during execution (or if the test
> is interrupted when running on the bot).
> See
http://stackoverflow.com/questions/1112343/how-do-i-capture-sigint-in-python
> for example.

Fixed with try+finally. Leave SIGINT for later.

[kjellander_webrtc, 2017-04-19 20:13:29]

On 2017/03/17 13:39:42, mandermo wrote:
>
https://codereview.webrtc.org/2741743002/diff/140001/webrtc/examples/androidt...
> File webrtc/examples/androidtests/video_quality_loopback_test.py (right):
> 
>
https://codereview.webrtc.org/2741743002/diff/140001/webrtc/examples/androidt...
> webrtc/examples/androidtests/video_quality_loopback_test.py:83: dev_appserver
=
> os.path.join(SRC_DIR, 'out', 'apprtc', 'google_appengine',
> On 2017/03/17 12:02:16, kjellander_webrtc wrote:
> > Add try+finally around the processes being launched so you ensure you
> terminate
> > them no matter what happens.
> > 
> > I guess you have two options here, defining all three variables
> (apprtc_process,
> > collider_process, reverseforwarder_process) to none outside a single
> try+finally
> > clause, or nest three different ones inside each other. I think the former
is
> > less cluttered.
> > 
> > I think you also should implement a SIGINT handler to protect against zombie
> > processes staying around if ctrl+c is pressed during execution (or if the
test
> > is interrupted when running on the bot).
> > See
> http://stackoverflow.com/questions/1112343/how-do-i-capture-sigint-in-python
> > for example.
> 
> Fixed with try+finally. Leave SIGINT for later.

Closing this in favor for https://codereview.webrtc.org/2825313002/