New issue
Advanced search Search tips

Issue 814802 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Simplify logic for matching --browser=exact on Android

Project Member Reported by perezju@chromium.org, Feb 22 2018

Issue description

The current logic is confusing and hard to follow.

It tries to find the correct backend settings by matching either the package or the apk name.

However, when the apk name matched, we then check that the package name *also* matches. Rendering this second possibility virtually useless.

It seems that the code got into its current form due to magnolia, when we wanted to skip the error message for unknown package names that *also* had unknown apk names.

But magnolia is no more, so it makes more sense to keep things simple and consistent, raising an error message for any apk with unknown package name.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/3bce15156364f929d3d4bf401cba629c8131f0b4

commit 3bce15156364f929d3d4bf401cba629c8131f0b4
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Fri Feb 23 09:26:09 2018

[Telemetry] Simplify code for --browser=exact on Android

The current logic is confusing and hard to follow.

This CL simplifies things to look up the correct backend settings by
package name only, which should be its true identifier, and ignoring
the apk file name.

Bug:  chromium:814802 
Change-Id: I757b9904e506357231f262acb3162dbae6f97ad8
Reviewed-on: https://chromium-review.googlesource.com/931506
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>

[modify] https://crrev.com/3bce15156364f929d3d4bf401cba629c8131f0b4/telemetry/telemetry/internal/backends/chrome/android_browser_finder.py

Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
Change broke catapult roll.

See e.g.:
https://chromium-review.googlesource.com/c/chromium/src/+/933628
https://chromium-review.googlesource.com/c/chromium/src/+/936471

Revert in progress.
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/38ba657f018acf920c1c52ad80dc520711f48720

commit 38ba657f018acf920c1c52ad80dc520711f48720
Author: Juan Antonio Navarro Pérez <perezju@chromium.org>
Date: Mon Feb 26 11:51:13 2018

Revert "[Telemetry] Simplify code for --browser=exact on Android"

This reverts commit 3bce15156364f929d3d4bf401cba629c8131f0b4.

Reason for revert: Broke catapult autoroll.

Original change's description:
> [Telemetry] Simplify code for --browser=exact on Android
> 
> The current logic is confusing and hard to follow.
> 
> This CL simplifies things to look up the correct backend settings by
> package name only, which should be its true identifier, and ignoring
> the apk file name.
> 
> Bug:  chromium:814802 
> Change-Id: I757b9904e506357231f262acb3162dbae6f97ad8
> Reviewed-on: https://chromium-review.googlesource.com/931506
> Reviewed-by: Ned Nguyen <nednguyen@google.com>
> Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>

TBR=perezju@chromium.org,nednguyen@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  chromium:814802 
Change-Id: Ic7badef06635dc48a8429b775fc806b2bc951064
Reviewed-on: https://chromium-review.googlesource.com/936742
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>

[modify] https://crrev.com/38ba657f018acf920c1c52ad80dc520711f48720/telemetry/telemetry/internal/backends/chrome/android_browser_finder.py

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/e06940ae3be9432ecf4542d80cdea7a99970d60d

commit e06940ae3be9432ecf4542d80cdea7a99970d60d
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Thu Mar 01 06:41:53 2018

Reland "[Telemetry] Simplify code for --browser=exact on Android"

The current logic is confusing and hard to follow.

This CL simplifies things to look up the correct backend settings by
package name only, which should be its true identifier, and ignoring
the apk file name.

Bug:  chromium:814802 
Change-Id: I9231c28fbc4a46752c60d8ac1bf01ac9ef482ec5
Reviewed-on: https://chromium-review.googlesource.com/941323
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/e06940ae3be9432ecf4542d80cdea7a99970d60d/telemetry/telemetry/internal/backends/chrome/android_browser_finder_unittest.py
[modify] https://crrev.com/e06940ae3be9432ecf4542d80cdea7a99970d60d/telemetry/telemetry/internal/backends/chrome/android_browser_finder.py

Status: Fixed (was: Assigned)

Comment 7 by benhenry@google.com, Jan 16 (6 days ago)

Components: Test>Telemetry

Comment 8 by benhenry@google.com, Jan 16 (6 days ago)

Components: -Speed>Telemetry

Sign in to add a comment