New issue
Advanced search Search tips

Issue 654569 link

Starred by 3 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocked on:
issue 611756
issue 653328
issue 654589



Sign in to add a comment

Need continuous test coverage of --site-per-process mode on Android: gtest-based tests

Project Member Reported by lukasza@chromium.org, Oct 10 2016

Issue description

Let's use this bug to track attempts to run gtest-based tests (i.e. content_browsertests, components_unittests) on Android, in --site-per-process mode.
 
Based on

    https://chromium.googlesource.com/chromium/src/+/master/docs/android_test_instructions.md

I've tried to do the following:

    . build/android/envsetup.sh
    build/android/avd.py run # --api-level=23
    ninja -C out/andr -j 1500 -l 15 content_browsertests
    out/andr/bin/run_content_browsertests --gtest_filter=*CrossSiteTransferTest*

This succeeded - it run the tests + the tests passed.

OTOH, at this point I have no idea how to pass the --site-per-process command line flag to the test executable.

Okay, I can pass --site-per-process to test_runner.py with: --test-arguments=--site-per-process

And I think, I should be able to pass a filter file (i.e. testing/buildbot/filters/site-per-process.content_browsertests.filter) using --gtest-filter-file switch.  Unfortunately, this switch doesn't seem to support comments (I think this switch will concatenate all lines from the file using ":" - this won't work if some of the lines represent comments).
Blockedon: 654589
For the record, I got 9 failing tests in a local run against the Chromium emulator:

MidiBrowserTest.RequestMIDIAccess
MidiBrowserTest.SubscribeAll
NavigationControllerBrowserTest.FrameNavigationEntry_NewSubframe
NavigationControllerBrowserTest.ReloadOriginalRequest
RequestDataResourceDispatcherHostBrowserTest.BasicCrossSite
SitePerProcessBrowserTest.CrossSiteIframe
SitePerProcessBrowserTest.NavigateRemoteFrame
SitePerProcessBrowserTest.RFPHDestruction
TracingControllerTest.DisableRecordingStoresMetadata
Owner: lukasza@chromium.org
Status: Started (was: Untriaged)
It seems that other chromium.*.json configs for android, pass arguments understood by build/android/test_runner.py (e.g. --shard-timeout) via gtest_tests.args entry.  So - it seems that for Site Isolation Android I should try doing the same thing:

    "gtest_tests": [
      {
        "args": [
          "--gtest-filter-file=../../testing/buildbot/filters/site-per-process.android.content_browsertests.filter",
          "--test-arguments=--site-per-process"
        ],
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bf2f3855e7e22042ad5d11e785b13d6e7c84127a

commit bf2f3855e7e22042ad5d11e785b13d6e7c84127a
Author: lukasza <lukasza@chromium.org>
Date: Wed Oct 19 23:35:15 2016

Add --site-per-process flavor of //content tests to Site Isolation Android bot.

BUG=654569

Review-Url: https://chromiumcodereview.appspot.com/2431793002
Cr-Commit-Position: refs/heads/master@{#426336}

[modify] https://crrev.com/bf2f3855e7e22042ad5d11e785b13d6e7c84127a/content/test/BUILD.gn
[modify] https://crrev.com/bf2f3855e7e22042ad5d11e785b13d6e7c84127a/testing/buildbot/chromium.fyi.json

Hmmm... there are some problems with --gtest-filter-file in the CL from #c6:

I    0.003s Main  command: /b/swarm_slave/w/irXe9E6P/build/android/test_runner.py gtest --suite content_browsertests --isolate-file-path /b/swarm_slave/w/irXe9E6P/out/Release/gen.runtime/content/test/content_browsertests.device.isolate --output-directory /b/swarm_slave/w/irXe9E6P/out/Release --logcat-output-file /b/swarm_slave/w/iohq5Dq6/logcats --target-devices-file /b/swarm_slave/w/bot_fileQfYAT3.json -v --gtest-filter-file=../../testing/buildbot/filters/site-per-process.content_browsertests.filter --test-arguments=--site-per-process --test-launcher-summary-output=/b/swarm_slave/w/iohq5Dq6/output.json
...
IOError: [Errno 2] No such file or directory: '/b/swarm_slave/w/irXe9E6P/testing/buildbot/filters/site-per-process.content_browsertests.filter'


I am not sure how to explain the behavior observed above.

Hypothesis #1: Wrong path (e.g. too many or too little "..").

This seems unlikely, as the path to the filter file is consistent with the path to other files from the repro (e.g. with the path to test_runner.py OR with the path to out/Release).

Hypothesis #2: The filter file is missing from the bot.

I am not sure how to ensure that the file gets copied to the bot.  FWIW, chromium.fyi.json says [1] "override_isolate_target":"content_browsertests" - I assume that this is fine.  I also note that the CL from #c6 did add [2] the filter file in BUILD.gn (when |is_android|, which I think should be present given the args.gn from the bot [3]).

[1] https://chromium.googlesource.com/chromium/src/+/72238d998cb2313fed81ca100c1405725c226b64/testing/buildbot/chromium.fyi.json#11391

[2] https://chromiumcodereview.appspot.com/2431793002/patch/20001/30001

[3] https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Android/builds/319/steps/generate_build_files/logs/stdio
Cc: bpastene@chromium.org
bpastene@, would you have any guidance on how to debug the issue above?  Is it possible for me to log in (e.g. via ssh) into the bot to double-check presence of site-per-process.content_browsertests.filter file during test steps?
Cc: jbudorick@chromium.org
Sure, I added you to the group that grants ssh access. You can follow instructions here  (they may be a bit outdated) to ssh on to the bot and poke around: https://chrome-internal.googlesource.com/infra/infra_internal/+/master/doc/ssh.md

It might be worth mentioning that the test runner generates wrapper python scripts for each test and places them in out/bin (eg src/out/bin/run_content_browsertests), which itself calls the actual test runner. Although, that would need the same number of '..'s, so maybe not relevant...

+ jbudorick@ in case he's ever seen problems with relative paths passed to android's test runner via testing/buildbot specs.
It's not getting isolated. You need to add android here: https://codesearch.chromium.org/chromium/src/content/test/BUILD.gn?rcl=0&l=673
Thanks jbudorick@ (and alexmos@ offline) for pointing out an issue with the BUILD.gn changes from r426336 - I have a fix proposal at https://chromiumcodereview.appspot.com/2434983003
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff56030a9b90a903622b1563dd7e56998d58e30a

commit ff56030a9b90a903622b1563dd7e56998d58e30a
Author: lukasza <lukasza@chromium.org>
Date: Fri Oct 21 16:50:29 2016

Really include *.content_browsertests.filter in data dependencies of Android.

r426336 intended to add a data dependency for
site-per-process.content_browsertests.filter on Android builds.  The CL
missed the fact that the modified line (where "|| is_android" was added)
is nested in a long if that says "if (!is_android)".  Doh...
This CL tries to ensure that the dependency is *really* there.

The CL also refactors away nested ifs and ifs with negative conditions.
Hopefully this will help avoid similar confusion in the future.

BUG=654569

Review-Url: https://chromiumcodereview.appspot.com/2434983003
Cr-Commit-Position: refs/heads/master@{#426817}

[modify] https://crrev.com/ff56030a9b90a903622b1563dd7e56998d58e30a/content/test/BUILD.gn

Description: Show this description
Summary: Need continuous test coverage of --site-per-process mode on Android: gtest-based tests (was: Need continuous test coverage of --site-per-process mode on Android: content_browsertests)
Blockedon: 611756
Note that running browser_tests with --site-per-process flag is blocked on issue 611756 (which track ability to run (most) browser_tests on Android).
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1c13ffe7c4cbf8887dfeac1467df19131aaa521c

commit 1c13ffe7c4cbf8887dfeac1467df19131aaa521c
Author: lukasza <lukasza@chromium.org>
Date: Tue Oct 25 16:49:52 2016

Add components_*tests and blimp_unittests to Site Isolation Android bot.

This is an attempt to add more tests run by Site Isolation Linux bot to
the set of tests run by Site Isolation Android bot.  Unfortunately, the
following test targets are not built and run on Android:
- app_shell_unittests, nacl_loader_unittests, sync_integration_tests
- browser_tests, interactive_ui_tests, unit_tests
- extensions_browsertests, extensions_unittests

Therefore, only 3 test targets are being added to Site Isolation Android
bot in this CL:
- components_browsertests
- components_unittests
- blimp_unittests

BUG=654569

Review-Url: https://codereview.chromium.org/2445853002
Cr-Commit-Position: refs/heads/master@{#427381}

[modify] https://crrev.com/1c13ffe7c4cbf8887dfeac1467df19131aaa521c/testing/buildbot/chromium.fyi.json

Project Member

Comment 17 by bugdroid1@chromium.org, Nov 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1f3f8c66d6efc3e759550eaa899757fc34307cfc

commit 1f3f8c66d6efc3e759550eaa899757fc34307cfc
Author: lukasza <lukasza@chromium.org>
Date: Tue Nov 08 16:53:56 2016

Fixing how --gtest-filter-file handles negative patterns.

When authoring the README.md file in https://crrev.com/2472153002, I've
noticed that test_runner.py doesn't properly handle negative patterns
from the filter files.  This CL fixes this and adds some unit tests
around this.  The changes are based on the understanding of the desired
format of --gtest_filter that is based on:

1) the format used in chromium.fyi.json prior to introducing
   testing/buildbot/filters directory - for example notice multiple
   patterns, but only a single '-' character in:
https://chromium.googlesource.com/chromium/src/+/e443d6647254776e868dce6a596459fdf75ebad9/testing/buildbot/chromium.fyi.json#5690

2) gtest documentation at
https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#running-a-subset-of-the-tests

BUG=654569

Review-Url: https://codereview.chromium.org/2477813002
Cr-Commit-Position: refs/heads/master@{#430634}

[modify] https://crrev.com/1f3f8c66d6efc3e759550eaa899757fc34307cfc/build/android/pylib/gtest/gtest_test_instance.py
[modify] https://crrev.com/1f3f8c66d6efc3e759550eaa899757fc34307cfc/build/android/pylib/gtest/gtest_test_instance_test.py

Owner: ----
Status: Available (was: Started)
The main left here is trying to also cover browser_tests, but this is largely blocked on issue 611756.
Project Member

Comment 19 by sheriffbot@chromium.org, Feb 21 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

Comment 20 by nasko@chromium.org, Feb 21 2018

Labels: -Hotlist-Recharge-Cold
This is still something we want to do and it looks it is blocked on 611756. Removing Hotlist-Recharge-Cold.

Sign in to add a comment