Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(53)

Issue 7558011: Finish deflaking the net-internals prerender view test and split it in two. (Closed)

Created:
9 years, 4 months ago by mmenke
Modified:
9 years, 4 months ago
Reviewers:
eroman
CC:
chromium-reviews, eroman, mmenke, Paweł Hajdan Jr.
Visibility:
Public.

Description

Deflake and clean up the net-internals prerender view test. It was occasionally timing out because I wasn't decreasing BrowserBridge's polling timer. That is now fixed. Along with some prerender history fixes, this should deflake the test. I've also split the net-internals prerender view test into two tests, one for the success case and one for the failure case. This simplifies the test a little, and now the success case visits the page. BUG=91799 TEST=NetInternalsTest.NetInternalsPrerenderViewSucceed, NetInternalsTest.NetInternalsPrerenderViewFail Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=96396

Patch Set 1 : '' #

Patch Set 2 : Fix final_status being set to FINAL_STATUS_MAX #

Patch Set 3 : Fix capitalization, remove file submitted for review separately. #

Patch Set 4 : Update comments. #

Total comments: 4

Patch Set 5 : Response to comments. #

Patch Set 6 : Update comments #

Patch Set 7 : Needless sync #

Unified diffs Side-by-side diffs Delta from patch set Stats (+139 lines, -69 lines) Patch
M chrome/browser/ui/webui/net_internals_ui_browsertest.cc View 1 2 3 4 5 6 5 chunks +67 lines, -7 lines 0 comments Download
M chrome/test/data/webui/net_internals/net_internals_test.js View 1 2 3 4 5 6 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/test/data/webui/net_internals/prerender_view.js View 1 2 3 4 5 6 6 chunks +69 lines, -60 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
mmenke
9 years, 4 months ago (2011-08-09 18:34:24 UTC) #1
eroman
lgtm http://codereview.chromium.org/7558011/diff/12001/chrome/test/data/webui/net_internals/prerender_view.js File chrome/test/data/webui/net_internals/prerender_view.js (right): http://codereview.chromium.org/7558011/diff/12001/chrome/test/data/webui/net_internals/prerender_view.js#newcode33 chrome/test/data/webui/net_internals/prerender_view.js:33: // We've added the prefetch link for |url|, ...
9 years, 4 months ago (2011-08-09 18:41:54 UTC) #2
mmenke
9 years, 4 months ago (2011-08-09 18:49:39 UTC) #3
Thanks.

I'm hoping to land tests for a view or two a week until we have a fairly
complete set.  Not sure about testing any other files directly, other than
exporting/importing log files.

http://codereview.chromium.org/7558011/diff/12001/chrome/test/data/webui/net_...
File chrome/test/data/webui/net_internals/prerender_view.js (right):

http://codereview.chromium.org/7558011/diff/12001/chrome/test/data/webui/net_...
chrome/test/data/webui/net_internals/prerender_view.js:33: // We've added the
prefetch link for |url|, openned a new tab if needed,
On 2011/08/09 18:41:54, eroman wrote:
> spelling: openned

Done.

http://codereview.chromium.org/7558011/diff/12001/chrome/test/data/webui/net_...
chrome/test/data/webui/net_internals/prerender_view.js:44: * @param {string} url
URL to prerendered.
On 2011/08/09 18:41:54, eroman wrote:
> "to be prerendered"

Done.

Powered by Google App Engine
This is Rietveld 408576698