New issue
Advanced search Search tips

Issue 720634 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Task



Sign in to add a comment

Add Client LoFi ChromeDriver integration tests

Project Member Reported by sclit...@chromium.org, May 10 2017

Issue description

We should have a ChromeDriver integration tests for Client LoFi to make sure that the full feature works. We should have one test that doesn't go through the DRP, and one test that does.
 

Comment 1 by bengr@chromium.org, Nov 29 2017

Labels: -M-60 M-65
Do we have these yet? 
Not yet. The feature is fully unittested, and we do have a chromedriver test that ensures the intervention header is added, but we don't yet have chromedriver tests for the full thing. I don't think these would be too tricky to add, I've just had higher priority things taking precedence.

Comment 3 by efoo@chromium.org, Dec 5 2017

Components: Blink>Previews

Comment 4 by efoo@chromium.org, Dec 5 2017

Components: -UI>Browser>Previews
I still need to do these. Refreshed during triage.

Comment 6 by bengr@chromium.org, Mar 21 2018

Refreshed during triage.
Refreshed during triage.

Comment 8 by bengr@chromium.org, May 21 2018

Why is a core integration test for a launched feature not a high priority?
I'm wondering if a chrome driver tests makes sense or whether a UI automator test makes sense, either way, are there any updates here?
Would the browser test suffice here? It's possible to simulate right clicks in the browser tests. It's also possible to click in infobars.
https://cs.chromium.org/search/?q=f:browser.*test+right.*click&sq=package:chromium&type=cs

If the goal is to somehow test the integration between client side lofi and server side lofi, then we would need to write an UI automator test.
I think the goal is to actually verify a placeholder was shown. If we can do that with UMA in a browsertest that would be sufficient IMO.
Labels: -M-65
Status: Started (was: Assigned)
The purpose of this test is to ensure that the DRP doesn't break Client Lo-Fi image range requests, if for some reason they get sent through the DRP. For example, set flags to disable server previews but allow Client Lo-Fi and force a slow ECT, then load a non-SSL page with an image on it, expecting the placeholder image to be shown.

Client Lo-Fi already has several unit tests and browser tests to ensure that the core functionality works.

This bug fell off my radar, I'll tackle it today though.
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 11

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

commit 051c824d017e9a1ffdd387118503f8b7bd810066
Author: Scott Little <sclittle@chromium.org>
Date: Wed Jul 11 17:59:00 2018

Added integration test for interaction between Client LoFi and the DRP.

Added an integration test that checks that range requests sent by
Client-side LoFi Previews that go through the Data Reduction Proxy are
responded to properly by the Data Reduction Proxy, and correctly
recognized as Client LoFi responses by Chrome.

Bug:  720634 
Change-Id: Iea40a0c60f96624733da1ea478f6ba443de5c454
Reviewed-on: https://chromium-review.googlesource.com/1130938
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Scott Little <sclittle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574243}
[modify] https://crrev.com/051c824d017e9a1ffdd387118503f8b7bd810066/tools/chrome_proxy/webdriver/lofi.py

Status: Fixed (was: Started)

Sign in to add a comment