New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 673194 link

Starred by 0 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

unify functionality of desktop gtest tests and mobile gtest-based tests

Project Member Reported by dpranke@chromium.org, Dec 12 2016

Issue description

In comment #8 of bug 667004, phajdan.jr@ wrote:

> This is on Android, so the good things about desktop gtest launcher you've 
> mentioned don't necessarily applied to it (FWIW: I encouraged some unification 
> so we'd get the same benefits on Android, and Android people declined).

Paweł, can you expand on what this functionality might be? We should make sure the test launchers have the same interface and the same functionality regardless of whether or not they share implementations. (Feel free to assign back to me once you've answered).

We should also figure out if the same applies to iOS or not.
 
This is something we should be working on in Q1.
Labels: Hotlist-Infra-Failures
Cc: phajdan.jr@chromium.org phajdan@google.com
Owner: dpranke@chromium.org
Some examples (I may need to think more for a more comprehensive list):

1) robust concurrency ; base/test/launcher uses battle-tested primitives from Chrome; historically python has been prone to weird hangs or races; it's also more tricky to do low-level things like fork/exec properly

2) robust timeouts ; also proper handling of test results in face of timeouts (i.e. the launcher should not crash, and should provide detailed results saying which tests timed out)

3) uniform command-line interface ; it's confusing when different launchers have different flags and even syntax for e.g. test filters

4) uniform disabling of tests ; similar to above: it's really cumbersome to disable tests for an unfamiliar test harness
wrt Android:

1) concurrency: on a given device, we have a lot to gain here. AFAIU fork is the reason why we don't currently use most of b/t/l/, though (https://codesearch.chromium.org/chromium/src/base/test/launcher/unit_test_launcher.cc?rcl=e7b734da304387bdc790c50e47024698982c71a8&l=186), so there could be some challenges here. (Haven't tried, but I'm guessing forked processes would at least run into issues with crossing back over the JNI.)

2) timeouts: we do this to some extent, but I would imagine b/t/l/'s support is more robust. This would also be a welcome gain.

3) CLI: I believe we support a subset of the gtest flags and the same filter syntax.

4) disabling: mobile gtests are *mostly* disabled via the same DISABLED_ mechanism as elsewhere. We still have a few filter files left in build/android/pylib/gtest/filter (https://bugs.chromium.org/p/chromium/issues/detail?id=339980) but not many.

I would also add:

5) results: no more rolling a duplicate implementation (https://codesearch.chromium.org/chromium/src/build/android/pylib/results/json_results.py?rcl=aa8ec6485091dda9862eb62960b881a60ef9e132&l=16) of the gtest JSON results generation.

I think there are some things that'd be great to defer to b/t/l/, but I do think we'll need to continue using the python launcher for the behavior that requires transferring things to devices or work that spans devices.
Owner: ----
Status: Available (was: Assigned)
Ack, thanks for the feedback. 

At this point, despite what I wrote in comment #2, I kinda doubt we'll get to this in Q1.
Project Member

Comment 6 by sheriffbot@chromium.org, Feb 16 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment