HostedAppTest should be parameterized for bookmark/hosted versions |
||
Issue descriptionSome tests apply to hosted apps, some apply to bookmark apps, and some are duplicated between the two (e.g., ShouldShowLocationBarForBookmarkApp and ShouldShowLocationBarForHostedApp are identical other than the setup). Almost all tests apply equally to both, so we should parameterize the test and remove duplicated logic. Context: I am changing the behaviour of showing location bar and I wanted to be sure it affects bookmark and hosted apps correctly. Not all of the tests covered all the cases (e.g., there is only a "WithoutWWW" test for hosted apps, and "ForHTTPApp" and "ForHTTPSApp" tests for bookmark apps; I believe it applies equally to both).
,
Feb 12 2018
Ah never mind. I add a parameter for the PWA flag, not where the actual tests run.
,
Feb 12 2018
CL is up: https://crrev.com/c/913188. Yeah, there's already a parameter, which made it hard to add a second one, but I did. Do you think this is the right direction? Discuss on CL. (Remember to like, share and subscribe.)
,
Feb 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a133f2225f885e5256d577f4eff6aea9ed1929d commit 4a133f2225f885e5256d577f4eff6aea9ed1929d Author: Matt Giuca <mgiuca@chromium.org> Date: Wed Feb 14 04:12:13 2018 HostedAppTest: Parameterize all tests for bookmark/hosted app versions. Previously, some tests applied to hosted apps, some applied to bookmark apps, and some were duplicated between the two. Almost all tests apply equally to both, so we parameterize the test and remove duplicated logic. Bug: 811175 Change-Id: I943e2ce49f3a62354121dfd83c3c9028776b8198 Reviewed-on: https://chromium-review.googlesource.com/913188 Commit-Queue: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Ben Wells <benwells@chromium.org> Cr-Commit-Position: refs/heads/master@{#536642} [modify] https://crrev.com/4a133f2225f885e5256d577f4eff6aea9ed1929d/chrome/browser/ui/extensions/hosted_app_browsertest.cc
,
Feb 14 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by ortuno@chromium.org
, Feb 12 2018