New issue
Advanced search Search tips

Issue 865339 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug


Participants' hotlists:
Hotlist-Tast


Sign in to add a comment

tast: Decouple ARC logic from chrome package

Project Member Reported by nya@chromium.org, Jul 19

Issue description

Currently ARC tests request to enable ARC by passing some options to chrome.New:

 cr, err := chrome.New(ctx, chrome.ARCEnabled())

We will have difficulties as we add more features to ARC --- for example, this design makes it difficult to return some objects on ARC initialization.

I propose to decouple ARC logic from Chrome. With new API, ARC tests will begin with something like this:

 cr, err := chrome.New(ctx)
 if err != nil { ... }
 defer cr.Close()

 a, err := arc.Start(ctx, cr)
 if err != nil { ... }
 defer a.Close()

 ... use a to interact with ARC ...

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/82148163069e04484ba670ccf939b1478486dc66

commit 82148163069e04484ba670ccf939b1478486dc66
Author: Shuhei Takahashi <nya@chromium.org>
Date: Fri Jul 20 12:30:57 2018

arc: Decouple ARC logic from chrome package.

As we are adding more features to ARC package, mixing ARC logic
and Chrome logic in chrome package is not desirable.

This change isolates ARC logic from chrome package. From now on,
ARC tests need to call arc.Start to enable ARC, rather than
passing some argument to chrome.New.

One benefit of the new API is that we can return ARC object from
arc.Start to keep some handles. Soon we are adding logcat process
handle there to save Android logs.

BUG= chromium:865339 
TEST=tast -verbose run caroline-DUT arc.Boot

Change-Id: I120ec277194fe4768012f861b65197d83222941e
Reviewed-on: https://chromium-review.googlesource.com/1143103
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Shuhei Takahashi <nya@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>

[modify] https://crrev.com/82148163069e04484ba670ccf939b1478486dc66/src/chromiumos/tast/local/bundles/cros/arc/intent_forward.go
[modify] https://crrev.com/82148163069e04484ba670ccf939b1478486dc66/src/chromiumos/tast/local/chrome/chrome.go
[modify] https://crrev.com/82148163069e04484ba670ccf939b1478486dc66/src/chromiumos/tast/local/bundles/cros/arc/boot.go
[modify] https://crrev.com/82148163069e04484ba670ccf939b1478486dc66/src/chromiumos/tast/local/bundles/cros/arc/downloads.go
[modify] https://crrev.com/82148163069e04484ba670ccf939b1478486dc66/src/chromiumos/tast/local/arc/adb.go
[modify] https://crrev.com/82148163069e04484ba670ccf939b1478486dc66/src/chromiumos/tast/local/arc/boot_phase.go

Status: Fixed (was: Started)

Sign in to add a comment