Add browser tests for the resource_prefetch_predictor |
||||||
Issue descriptionThere are currently only unit tests, this needs higher level ones as well.
,
Sep 28 2016
,
Oct 12 2016
These tests is necessary to ensure that ResourcePrefetchPredictor really works and fetches pages subresources before first bytes of a main page arrive.
,
Nov 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e636e3bcc172f220dfadfd2121d238c9908185bb commit e636e3bcc172f220dfadfd2121d238c9908185bb Author: alexilin <alexilin@chromium.org> Date: Fri Nov 04 19:07:18 2016 predictors: Basic browsertest checks predictor learning. First stage for implementing ResourcePrefetchPredictor browsertests is to check that the prefetch database is really filled after some navigations in browser. Browsertest class provides convenient way to declare expected resources in the test and then checks expectations internally that allows to write readable tests. BUG= 650253 Review-Url: https://codereview.chromium.org/2449323005 Cr-Commit-Position: refs/heads/master@{#429966} [add] https://crrev.com/e636e3bcc172f220dfadfd2121d238c9908185bb/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc [modify] https://crrev.com/e636e3bcc172f220dfadfd2121d238c9908185bb/chrome/browser/predictors/resource_prefetch_predictor_test_util.cc [modify] https://crrev.com/e636e3bcc172f220dfadfd2121d238c9908185bb/chrome/browser/predictors/resource_prefetch_predictor_test_util.h [modify] https://crrev.com/e636e3bcc172f220dfadfd2121d238c9908185bb/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc [modify] https://crrev.com/e636e3bcc172f220dfadfd2121d238c9908185bb/chrome/test/BUILD.gn [add] https://crrev.com/e636e3bcc172f220dfadfd2121d238c9908185bb/chrome/test/data/predictors/html_subresources.html [modify] https://crrev.com/e636e3bcc172f220dfadfd2121d238c9908185bb/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter
,
Nov 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3250e71949fa7d226e9c843ee28277b988306229 commit 3250e71949fa7d226e9c843ee28277b988306229 Author: mathp <mathp@chromium.org> Date: Sat Nov 05 03:11:19 2016 Revert of predictors: Basic browsertest checks predictor learning. (patchset #9 id:160001 of https://codereview.chromium.org/2449323005/ ) Reason for revert: Created a flaky test: first occurrence at https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%281%29/builds/28933 Original issue's description: > predictors: Basic browsertest checks predictor learning. > > First stage for implementing ResourcePrefetchPredictor browsertests is to check > that the prefetch database is really filled after some navigations in browser. > Browsertest class provides convenient way to declare expected resources in the > test and then checks expectations internally that allows to write readable > tests. > > BUG= 650253 > > Committed: https://crrev.com/e636e3bcc172f220dfadfd2121d238c9908185bb > Cr-Commit-Position: refs/heads/master@{#429966} TBR=pasko@chromium.org,lizeb@chromium.org,alexilin@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 650253 Review-Url: https://codereview.chromium.org/2470143007 Cr-Commit-Position: refs/heads/master@{#430137} [delete] https://crrev.com/fe64f860fd727cae179210713faee87b6c9e9209/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc [modify] https://crrev.com/3250e71949fa7d226e9c843ee28277b988306229/chrome/browser/predictors/resource_prefetch_predictor_test_util.cc [modify] https://crrev.com/3250e71949fa7d226e9c843ee28277b988306229/chrome/browser/predictors/resource_prefetch_predictor_test_util.h [modify] https://crrev.com/3250e71949fa7d226e9c843ee28277b988306229/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc [modify] https://crrev.com/3250e71949fa7d226e9c843ee28277b988306229/chrome/test/BUILD.gn [delete] https://crrev.com/fe64f860fd727cae179210713faee87b6c9e9209/chrome/test/data/predictors/html_subresources.html [modify] https://crrev.com/3250e71949fa7d226e9c843ee28277b988306229/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter
,
Nov 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f2d707775a8184e7bd18d2334ff2a679f3a90439 commit f2d707775a8184e7bd18d2334ff2a679f3a90439 Author: alexilin <alexilin@chromium.org> Date: Tue Nov 08 10:01:15 2016 Reland of predictors: Basic browsertest checks predictor learning. Reason for reland: The reason of the test flakiness was fixed. ResourcePrefetchPredictor needs to be initialized before learn any data. Initialization starts when the first main frame load complete for given profile. In our case it was about:blank page after tab creation. This initialization requires work in DB thread to be done. So there was a race condition in which next navigation could appear earlier than initialization in DB thread will be done. This bug could be easily reproduced by adding sleep in ResourcePrefetchPredictorTables::GetAllData function. For now we explicitly check that initialization of ResourcePrefetchPredictor was done before any navigation. Original issue's description: > Revert of predictors: Basic browsertest checks predictor learning. (patchset #9 id:160001 of https://codereview.chromium.org/2449323005/ ) > > Reason for revert: > Created a flaky test: first occurrence at > > https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%281%29/builds/28933 > > Original issue's description: > > predictors: Basic browsertest checks predictor learning. > > > > First stage for implementing ResourcePrefetchPredictor browsertests is to check > > that the prefetch database is really filled after some navigations in browser. > > Browsertest class provides convenient way to declare expected resources in the > > test and then checks expectations internally that allows to write readable > > tests. > > > > BUG= 650253 > > > > Committed: https://crrev.com/e636e3bcc172f220dfadfd2121d238c9908185bb > > Cr-Commit-Position: refs/heads/master@{#429966} > > TBR=pasko@chromium.org,lizeb@chromium.org,alexilin@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 650253 > > Committed: https://crrev.com/3250e71949fa7d226e9c843ee28277b988306229 > Cr-Commit-Position: refs/heads/master@{#430137} BUG= 650253 Review-Url: https://codereview.chromium.org/2482833002 Cr-Commit-Position: refs/heads/master@{#430561} [modify] https://crrev.com/f2d707775a8184e7bd18d2334ff2a679f3a90439/chrome/browser/predictors/resource_prefetch_predictor.cc [modify] https://crrev.com/f2d707775a8184e7bd18d2334ff2a679f3a90439/chrome/browser/predictors/resource_prefetch_predictor.h [add] https://crrev.com/f2d707775a8184e7bd18d2334ff2a679f3a90439/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc [modify] https://crrev.com/f2d707775a8184e7bd18d2334ff2a679f3a90439/chrome/browser/predictors/resource_prefetch_predictor_test_util.cc [modify] https://crrev.com/f2d707775a8184e7bd18d2334ff2a679f3a90439/chrome/browser/predictors/resource_prefetch_predictor_test_util.h [modify] https://crrev.com/f2d707775a8184e7bd18d2334ff2a679f3a90439/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc [modify] https://crrev.com/f2d707775a8184e7bd18d2334ff2a679f3a90439/chrome/test/BUILD.gn [add] https://crrev.com/f2d707775a8184e7bd18d2334ff2a679f3a90439/chrome/test/data/predictors/html_subresources.html [modify] https://crrev.com/f2d707775a8184e7bd18d2334ff2a679f3a90439/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter
,
Nov 9 2016
Patched version of browsertests is still a little flaky. (see flakiness dashboard http://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=browser_tests&tests=ResourcePrefetchPredictorBrowserTest.*) There are three different sources of flakiness: 1. The browser made request to the same image twice (then expectations fails): ChromiumOS: https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20(1)/builds/29128/steps/browser_tests%20on%20Ubuntu-12.04/logs/stdio Mac: https://build.chromium.org/p/chromium.mac/builders/Mac10.9%20Tests/builds/31477/steps/browser_tests%20on%20Mac-10.9/logs/stdio Win: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20(1)/builds/59730/steps/browser_tests%20on%20Windows-7-SP1/logs/stdio 2. Request to image had a higher priority than we expected: Mac: https://build.chromium.org/p/chromium.mac/builders/Mac10.10%20Tests/builds/8928/steps/browser_tests%20on%20Mac-10.10/logs/stdio Win: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20(dbg)(1)/builds/54504/steps/browser_tests%20on%20Windows-7-SP1/logs/stdio 3. Unknown timeout error in the run where navigation didn't even started: Linux: https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/48689/steps/browser_tests%20on%20Ubuntu-12.04/logs/stdio It seems like issue 3 . is a tester problem but 1. 2. are real. I'm not sure how critical it is because it happens very rare. For now I don't know how to change our expectations in tests to avoid these problems (except to not check net::RequestPriority at all that helps with 2.).
,
Nov 10 2016
,
Nov 14 2016
,
Nov 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf5b8e0f7794b08c10af2bb682bee81978df84c5 commit bf5b8e0f7794b08c10af2bb682bee81978df84c5 Author: alexilin <alexilin@chromium.org> Date: Tue Nov 22 18:58:19 2016 predictors: Add browsertest that checks redirects. Make sure that ResourcePrefetchPredictor learns about subresources when a main resource has redirects. BUG= 650253 Review-Url: https://codereview.chromium.org/2519963002 Cr-Commit-Position: refs/heads/master@{#433932} [modify] https://crrev.com/bf5b8e0f7794b08c10af2bb682bee81978df84c5/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
,
Nov 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3fbb26424d252cec6fb1a20f94f47a6211dcf80e commit 3fbb26424d252cec6fb1a20f94f47a6211dcf80e Author: alexilin <alexilin@chromium.org> Date: Tue Nov 29 10:38:11 2016 predictors: Ignore repeating subresources while checking. This eliminates one of the sources of flakiness in ResourcePrefetchPredictorBrowserTest. It seems impossible to avoid an emergence of several requests belonging to the same subresource but we could just ignore them as real code does. The real code also ignores repeating entries of the same subresource but doesn't modify collection passing to test observer. BUG= 650253 Review-Url: https://codereview.chromium.org/2529263003 Cr-Commit-Position: refs/heads/master@{#434951} [modify] https://crrev.com/3fbb26424d252cec6fb1a20f94f47a6211dcf80e/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
,
Nov 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2ccb52078094f6e0af7384ace1e4f14237a193b0 commit 2ccb52078094f6e0af7384ace1e4f14237a193b0 Author: alexilin <alexilin@chromium.org> Date: Wed Nov 30 14:13:41 2016 predictors: Delete unnecessary parameter from RequestHandler We don't need to pass the server as parameter to RequestHandler since crrev.com/2522663004 was landed. BUG= 650253 Review-Url: https://codereview.chromium.org/2545483002 Cr-Commit-Position: refs/heads/master@{#435252} [modify] https://crrev.com/2ccb52078094f6e0af7384ace1e4f14237a193b0/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
,
Dec 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7d03e6b973c673b3fcfd24c3e962f59875078fb8 commit 7d03e6b973c673b3fcfd24c3e962f59875078fb8 Author: alexilin <alexilin@chromium.org> Date: Tue Dec 06 10:43:59 2016 predictors: Add browsertests that check fetching through javascript. Make sure that ResourcePrefetchPredictor awares about javascript initiated requests + test for iframe. BUG= 650253 Review-Url: https://codereview.chromium.org/2540803002 Cr-Commit-Position: refs/heads/master@{#436566} [modify] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc [add] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/test/data/predictors/append_child.html [add] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/test/data/predictors/append_child.js [add] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/test/data/predictors/document_write.html [add] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/test/data/predictors/document_write.js [add] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/test/data/predictors/html_iframe.html [modify] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/test/data/predictors/html_subresources.html [add] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/test/data/predictors/inner_html.html [add] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/test/data/predictors/inner_html.js [add] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/test/data/predictors/xhr.html [add] https://crrev.com/7d03e6b973c673b3fcfd24c3e962f59875078fb8/chrome/test/data/predictors/xhr.js
,
Dec 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/23b1517fb1deb148503f67f1bc9f38283f4e89a0 commit 23b1517fb1deb148503f67f1bc9f38283f4e89a0 Author: ahemery <ahemery@chromium.org> Date: Mon Dec 12 15:29:49 2016 Summary ----- Allows for custom comparison of results and exotic navigation setup. Details ----- - New browser tests will be needed to test edge cases. For example verifying behavior during cross tab navigation. Useful for regression with PlzNavigate. - To be able to add the test cases we need to have some more customization than what the current test framework provides. Specifically, partial comparison and new navigation types. Implemented that. BUG= 650253 Review-Url: https://codereview.chromium.org/2561493003 Cr-Commit-Position: refs/heads/master@{#437877} [modify] https://crrev.com/23b1517fb1deb148503f67f1bc9f38283f4e89a0/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
,
Dec 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/54585e4c963fadabad06fd785ba30df548f9ade6 commit 54585e4c963fadabad06fd785ba30df548f9ade6 Author: alexilin <alexilin@chromium.org> Date: Mon Dec 12 23:38:58 2016 predictors: Fuzzy comparison of request priority for images. The browser tests for ResourcePrefetchPredictor check that the actual request priority is equal to the expected value. But for the images the priority cannot be precisely defined because there is a race between the preload scanner issuing fetch requests and frame view performing layout and promoting priorities of all visible images. BUG= 650253 , 673028 Review-Url: https://codereview.chromium.org/2569723002 Cr-Commit-Position: refs/heads/master@{#437958} [modify] https://crrev.com/54585e4c963fadabad06fd785ba30df548f9ade6/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
,
Dec 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c7c1e0389fa3b0d68e9b940b0929e2e3dd4bde9a commit c7c1e0389fa3b0d68e9b940b0929e2e3dd4bde9a Author: alexilin <alexilin@chromium.org> Date: Mon Dec 19 17:31:28 2016 predictors: Add browsertest that tests prefetching. Before this we only had tests for filling predictor database. This CL adds test that prefills database and then activates prefetching on the same web page. Then it checks that cache actually has been populated by prefetched resources. BUG= 650253 Review-Url: https://codereview.chromium.org/2553083002 Cr-Commit-Position: refs/heads/master@{#439494} [modify] https://crrev.com/c7c1e0389fa3b0d68e9b940b0929e2e3dd4bde9a/chrome/browser/predictors/resource_prefetch_predictor.cc [modify] https://crrev.com/c7c1e0389fa3b0d68e9b940b0929e2e3dd4bde9a/chrome/browser/predictors/resource_prefetch_predictor.h [modify] https://crrev.com/c7c1e0389fa3b0d68e9b940b0929e2e3dd4bde9a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc [modify] https://crrev.com/c7c1e0389fa3b0d68e9b940b0929e2e3dd4bde9a/chrome/browser/predictors/resource_prefetcher_manager.cc
,
Dec 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f15203e94ad5245c9ab50fbc0e1385ef403d0903 commit f15203e94ad5245c9ab50fbc0e1385ef403d0903 Author: alexilin <alexilin@chromium.org> Date: Thu Dec 22 11:09:33 2016 predictors: Wait for the cache cleaning synchronously. chrome::ClearCache() is asynchronous so there was possibility that cache is cleared after prefetching causing flakiness. This problem was almost hidden by that fact that the last navigation (after prefetching) also caused prefetching. So the probability that cache is cleared after the second prefetching was even less. This CL also changes command line flag to disable the second prefetching because it only tangles the tests. BUG= 650253 Review-Url: https://codereview.chromium.org/2596893002 Cr-Commit-Position: refs/heads/master@{#440379} [modify] https://crrev.com/f15203e94ad5245c9ab50fbc0e1385ef403d0903/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
,
Dec 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/820fc0653f57cfa4d75396b2606116430cb67aeb commit 820fc0653f57cfa4d75396b2606116430cb67aeb Author: alexilin <alexilin@chromium.org> Date: Tue Dec 27 08:20:53 2016 predictors: ResourcePrefetchPredictorBrowserTest improvements. This CL contains 4 minor changes bundled together. - ResourceSummary flags refactoring. Now they have more clean and strict meanings + additional descriptions in comments. - New check at HTTP server side that there is no resource requests after prefetch making the tests more strict. - New test with "Cache-control: no-cache" resources. - Respectful handling of validation requests. BUG= 650253 Review-Url: https://codereview.chromium.org/2592243003 Cr-Commit-Position: refs/heads/master@{#440740} [modify] https://crrev.com/820fc0653f57cfa4d75396b2606116430cb67aeb/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
,
Jan 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7e5dcb16fb03140fb729fbf58b03bb02ebe9df0e commit 7e5dcb16fb03140fb729fbf58b03bb02ebe9df0e Author: alexilin <alexilin@chromium.org> Date: Mon Jan 09 16:19:59 2017 predictors: browser test for client-side redirect. ResourcePrefetchPredictor doesn't receive the information from observers about redirects initiated from javascript. However such redirects initiate the new navigation that treated by predictor as usual. This browser test documents the current behavior that could be a subject of change. Also, this CL changes all existing redirect tests to be cross-host. Redirect to the same host could be ambiguous because of host-based learning. BUG= 650253 Review-Url: https://codereview.chromium.org/2609403007 Cr-Commit-Position: refs/heads/master@{#442259} [modify] https://crrev.com/7e5dcb16fb03140fb729fbf58b03bb02ebe9df0e/chrome/browser/predictors/resource_prefetch_predictor.h [modify] https://crrev.com/7e5dcb16fb03140fb729fbf58b03bb02ebe9df0e/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc [modify] https://crrev.com/7e5dcb16fb03140fb729fbf58b03bb02ebe9df0e/chrome/test/data/predictors/html_subresources.html [add] https://crrev.com/7e5dcb16fb03140fb729fbf58b03bb02ebe9df0e/chrome/test/data/predictors/javascript_redirect.html [add] https://crrev.com/7e5dcb16fb03140fb729fbf58b03bb02ebe9df0e/chrome/test/data/predictors/javascript_redirect.js
,
Jan 23 2017
Mark this as done because all basic use-cases were covered. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by alexilin@chromium.org
, Sep 27 2016