Migrating tests to use EmbeddedTestServer (/content)

Migrating tests to use EmbeddedTestServer (/content)

- Moving tests to use EmbeddedTestServer where possible.
- net::test_server::EmbeddedTestServer references to net::EmbeddedTestServer
- Using server.Start() instead of server.InitializeAndWaitUntilReady()
- Renaming test_server() to be spawned_test_server() or embedded_test_server()
- Enabling tests disabled due to SpawnedTestServer compatibility

BUG=496825
Committed: https://crrev.com/c3a9a1736ca602c624d7b2aaff555bd3ebf9f64b
Cr-Commit-Position: refs/heads/master@{#357645}

[svaldez, 2015-11-02 19:09:26]

svaldez@chromium.org changed reviewers:
+ davidben@chromium.org, mmenke@chromium.org

[mmenke, 2015-11-03 19:12:57]

https://codereview.chromium.org/1411073005/diff/1/content/browser/accessibili...
File content/browser/accessibility/site_per_process_accessibility_browsertest.cc
(right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/accessibili...
content/browser/accessibility/site_per_process_accessibility_browsertest.cc:72:
GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));
Should include embedded_test_server.h.

https://codereview.chromium.org/1411073005/diff/1/content/browser/download/do...
File content/browser/download/download_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/download/do...
content/browser/download/download_browsertest.cc:40: #include
"net/test/url_request/url_request_mock_http_job.h"
Why did you remove the spawned_test_server header?  This file is still using it.

https://codereview.chromium.org/1411073005/diff/1/content/browser/download/sa...
File content/browser/download/save_package_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/download/sa...
content/browser/download/save_package_browsertest.cc:9: #include
"content/shell/browser/shell.h"
include embedded_test_server.h

https://codereview.chromium.org/1411073005/diff/1/content/browser/site_per_pr...
File content/browser/site_per_process_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/site_per_pr...
content/browser/site_per_process_browsertest.cc:1499:
ASSERT_TRUE(embedded_test_server()->Start());
Not needed - it's already started.

https://codereview.chromium.org/1411073005/diff/1/content/browser/site_per_pr...
content/browser/site_per_process_browsertest.cc:1621:
ASSERT_TRUE(embedded_test_server()->Start());
Not needed.

https://codereview.chromium.org/1411073005/diff/1/content/browser/web_content...
File content/browser/web_contents/web_contents_view_aura_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_view_aura_browsertest.cc:256: if (url
== "about:blank")
Why did this work before?  Doesn't look like the old test server would have
worked here, either.

https://codereview.chromium.org/1411073005/diff/1/content/browser/webkit_brow...
File content/browser/webkit_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/webkit_brow...
content/browser/webkit_browsertest.cc:10: #include
"content/test/net/url_request_abort_on_end_job.h"
include embedded_test_server.h

https://codereview.chromium.org/1411073005/diff/1/content/browser/webui/web_u...
File content/browser/webui/web_ui_mojo_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/webui/web_u...
content/browser/webui/web_ui_mojo_browsertest.cc:32: #include
"third_party/mojo/src/mojo/public/js/constants.h"
include embedded_test_server.h

https://codereview.chromium.org/1411073005/diff/1/content/child/site_isolatio...
File content/child/site_isolation_stats_gatherer_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/child/site_isolatio...
content/child/site_isolation_stats_gatherer_browsertest.cc:15: #include
"content/shell/browser/shell.h"
include embedded_test_server.h

https://codereview.chromium.org/1411073005/diff/1/content/public/test/browser...
File content/public/test/browser_test_utils.h (right):

https://codereview.chromium.org/1411073005/diff/1/content/public/test/browser...
content/public/test/browser_test_utils.h:24: #include
"net/test/embedded_test_server/embedded_test_server.h"
Forward declare this?  Admittedly, it's a bit ugly (Since you need the forward
declare + typedef in this file, for now), but including this here makes it
easier to make files that aren't include-what-you-use clean.

https://codereview.chromium.org/1411073005/diff/1/content/renderer/resource_f...
File content/renderer/resource_fetcher_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/renderer/resource_f...
content/renderer/resource_fetcher_browsertest.cc:25: #include
"third_party/WebKit/public/web/WebView.h"
include embedded_test_server.h

[svaldez, 2015-11-03 19:33:16]

https://codereview.chromium.org/1411073005/diff/1/content/browser/accessibili...
File content/browser/accessibility/site_per_process_accessibility_browsertest.cc
(right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/accessibili...
content/browser/accessibility/site_per_process_accessibility_browsertest.cc:72:
GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));
On 2015/11/03 19:12:57, mmenke wrote:
> Should include embedded_test_server.h.

Done.

https://codereview.chromium.org/1411073005/diff/1/content/browser/download/do...
File content/browser/download/download_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/download/do...
content/browser/download/download_browsertest.cc:40: #include
"net/test/url_request/url_request_mock_http_job.h"
On 2015/11/03 19:12:57, mmenke wrote:
> Why did you remove the spawned_test_server header?  This file is still using
it.

Done.

https://codereview.chromium.org/1411073005/diff/1/content/browser/download/sa...
File content/browser/download/save_package_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/download/sa...
content/browser/download/save_package_browsertest.cc:9: #include
"content/shell/browser/shell.h"
On 2015/11/03 19:12:57, mmenke wrote:
> include embedded_test_server.h

Done.

https://codereview.chromium.org/1411073005/diff/1/content/browser/site_per_pr...
File content/browser/site_per_process_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/site_per_pr...
content/browser/site_per_process_browsertest.cc:1499:
ASSERT_TRUE(embedded_test_server()->Start());
On 2015/11/03 19:12:57, mmenke wrote:
> Not needed - it's already started.

Done.

https://codereview.chromium.org/1411073005/diff/1/content/browser/site_per_pr...
content/browser/site_per_process_browsertest.cc:1621:
ASSERT_TRUE(embedded_test_server()->Start());
On 2015/11/03 19:12:57, mmenke wrote:
> Not needed.

Done.

https://codereview.chromium.org/1411073005/diff/1/content/browser/web_content...
File content/browser/web_contents/web_contents_view_aura_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_view_aura_browsertest.cc:256: if (url
== "about:blank")
On 2015/11/03 19:12:57, mmenke wrote:
> Why did this work before?  Doesn't look like the old test server would have
> worked here, either.

Because the old test server returns a 200 page when it is unable to handle the
request.

https://codereview.chromium.org/1411073005/diff/1/content/browser/webkit_brow...
File content/browser/webkit_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/webkit_brow...
content/browser/webkit_browsertest.cc:10: #include
"content/test/net/url_request_abort_on_end_job.h"
On 2015/11/03 19:12:57, mmenke wrote:
> include embedded_test_server.h

Done.

https://codereview.chromium.org/1411073005/diff/1/content/browser/webui/web_u...
File content/browser/webui/web_ui_mojo_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/browser/webui/web_u...
content/browser/webui/web_ui_mojo_browsertest.cc:32: #include
"third_party/mojo/src/mojo/public/js/constants.h"
On 2015/11/03 19:12:57, mmenke wrote:
> include embedded_test_server.h

Done.

https://codereview.chromium.org/1411073005/diff/1/content/child/site_isolatio...
File content/child/site_isolation_stats_gatherer_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/child/site_isolatio...
content/child/site_isolation_stats_gatherer_browsertest.cc:15: #include
"content/shell/browser/shell.h"
On 2015/11/03 19:12:57, mmenke wrote:
> include embedded_test_server.h

Done.

https://codereview.chromium.org/1411073005/diff/1/content/public/test/browser...
File content/public/test/browser_test_utils.h (right):

https://codereview.chromium.org/1411073005/diff/1/content/public/test/browser...
content/public/test/browser_test_utils.h:24: #include
"net/test/embedded_test_server/embedded_test_server.h"
On 2015/11/03 19:12:57, mmenke wrote:
> Forward declare this?  Admittedly, it's a bit ugly (Since you need the forward
> declare + typedef in this file, for now), but including this here makes it
> easier to make files that aren't include-what-you-use clean.

Done.

https://codereview.chromium.org/1411073005/diff/1/content/renderer/resource_f...
File content/renderer/resource_fetcher_browsertest.cc (right):

https://codereview.chromium.org/1411073005/diff/1/content/renderer/resource_f...
content/renderer/resource_fetcher_browsertest.cc:25: #include
"third_party/WebKit/public/web/WebView.h"
On 2015/11/03 19:12:57, mmenke wrote:
> include embedded_test_server.h

Done.

[mmenke, 2015-11-03 19:34:06]

LGTM

[davidben, 2015-11-03 19:51:33]

lgtm

https://codereview.chromium.org/1411073005/diff/20001/content/public/test/bro...
File content/public/test/browser_test_utils.h (right):

https://codereview.chromium.org/1411073005/diff/20001/content/public/test/bro...
content/public/test/browser_test_utils.h:44: }
Now that you've included the header, there's no need to forward-declare like
that. (And, yeah, for this I think it's better to just include the header than
forward decl.)

[mmenke, 2015-11-03 20:03:01]

https://codereview.chromium.org/1411073005/diff/20001/content/public/test/bro...
File content/public/test/browser_test_utils.h (right):

https://codereview.chromium.org/1411073005/diff/20001/content/public/test/bro...
content/public/test/browser_test_utils.h:44: }
On 2015/11/03 19:51:33, davidben wrote:
> Now that you've included the header, there's no need to forward-declare like
> that. (And, yeah, for this I think it's better to just include the header than
> forward decl.)

I actually have the opposite opinion - we should remove the header (Because if
we go the other way around, we'll probably forget to remove the include in a
future CL, but this will break once we get rid of the test_server namespace).

[svaldez, 2015-11-03 20:06:36]

The CQ bit was checked by svaldez@chromium.org

[svaldez, 2015-11-03 20:06:37]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org,
davidben@chromium.org
Link to the patchset: https://codereview.chromium.org/1411073005/#ps40001
(title: "Removing unused header.")

[commit-bot: I haz the power, 2015-11-03 20:11:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1411073005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1411073005/40001

[commit-bot: I haz the power, 2015-11-03 20:17:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-03 20:17:08]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[svaldez, 2015-11-03 20:33:43]

The CQ bit was checked by svaldez@chromium.org

[svaldez, 2015-11-03 20:33:43]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org,
davidben@chromium.org
Link to the patchset: https://codereview.chromium.org/1411073005/#ps60001
(title: "Rebase.")

[commit-bot: I haz the power, 2015-11-03 20:37:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1411073005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1411073005/60001

[commit-bot: I haz the power, 2015-11-03 22:01:44]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-03 22:02:50]

Patchset 4 (id:??) landed as
https://crrev.com/c3a9a1736ca602c624d7b2aaff555bd3ebf9f64b
Cr-Commit-Position: refs/heads/master@{#357645}