Split up ios/web:test_support.

Split up ios/web:test_support.

No test files are in ios/web, so this creates BUILD.gn files
for the subdirectories. Update clients outside of ios/web to
depend on ios/web/public/test/*, instead of ios/web/test.

BUG=684616
TBR=msarda,droger

Review-Url: https://codereview.chromium.org/2898733003
Cr-Commit-Position: refs/heads/master@{#475962}
Committed: https://chromium.googlesource.com/chromium/src/+/66cc96f0414e2c1b91d177e33314a7fa1008d644

[baxley, 2017-05-22 23:34:35]

baxley@chromium.org changed reviewers:
+ eugenebut@chromium.org

[baxley, 2017-05-22 23:34:36]

I started with ios/web:test_support.

I'll do the EG test support, and the non-test code in separate CLs.

Let me know what you think!

[Eugene But (OOO till 7-30), 2017-05-23 00:15:11]

https://codereview.chromium.org/2898733003/diff/20001/ios/web/public/test/BUI...
File ios/web/public/test/BUILD.gn (right):

https://codereview.chromium.org/2898733003/diff/20001/ios/web/public/test/BUI...
ios/web/public/test/BUILD.gn:24:
"//ios/web/public/test/response_providers:test_support",
Is it possible to avoid circular dependencies by splitting BUILD files into
smaller source_sets?

https://codereview.chromium.org/2898733003/diff/20001/ios/web/public/test/fak...
File ios/web/public/test/fakes/BUILD.gn (right):

https://codereview.chromium.org/2898733003/diff/20001/ios/web/public/test/fak...
ios/web/public/test/fakes/BUILD.gn:18: allow_circular_includes_from = [
"//ios/web/test:test_support" ]
ditto

[baxley, 2017-05-23 21:03:38]

https://codereview.chromium.org/2898733003/diff/20001/ios/web/public/test/BUI...
File ios/web/public/test/BUILD.gn (right):

https://codereview.chromium.org/2898733003/diff/20001/ios/web/public/test/BUI...
ios/web/public/test/BUILD.gn:24:
"//ios/web/public/test/response_providers:test_support",
On 2017/05/23 00:15:11, Eugene But wrote:
> Is it possible to avoid circular dependencies by splitting BUILD files into
> smaller source_sets?

These are pretty sticky. I can do the first one, by moving http_server into a
separate target.

It's harder to break the dependency circle with ios/web/test:test_support.

ios/web/test:test_support uses web_test.h and web_test_with_web_state.h, plus
some more things which are in ios/web/public/test (and used in other tests).

ios/web/public/test then relies upon a decent amount of things in ios/web/test.

It may be more reasonable to make it work when the rest of ios/web is split up.
1. Keep this as is, then clean up later (has risk of not getting done :( )
2. Split things up just to make ios/web/test and ios/web/public/test clean (this
may require another round when splitting up ios/web).
3. Change this CL to do all of ios/web (will take a lot longer).

I think I prefer number 1 or 3, but want to see what you think.

https://codereview.chromium.org/2898733003/diff/20001/ios/web/public/test/fak...
File ios/web/public/test/fakes/BUILD.gn (right):

https://codereview.chromium.org/2898733003/diff/20001/ios/web/public/test/fak...
ios/web/public/test/fakes/BUILD.gn:18: allow_circular_includes_from = [
"//ios/web/test:test_support" ]
On 2017/05/23 00:15:11, Eugene But wrote:
> ditto

done!

[Eugene But (OOO till 7-30), 2017-05-23 22:19:32]

Generally your CL is a significant improvement, so one circular dependency is
probably fine. lgtm and thank you for doing this!

[baxley, 2017-05-25 20:41:07]

The CQ bit was checked by baxley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-25 20:41:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-25 20:47:39]

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

[commit-bot: I haz the power, 2017-05-25 20:47:40]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[baxley, 2017-05-25 22:30:07]

The CQ bit was checked by baxley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-25 22:31:25]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-26 00:34:04]

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

[commit-bot: I haz the power, 2017-05-26 00:34:06]

Dry run: This issue passed the CQ dry run.

[baxley, 2017-05-26 16:47:53]

The CQ bit was checked by baxley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-26 16:48:16]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[baxley, 2017-05-26 16:50:25]

On 2017/05/23 22:19:32, Eugene But wrote:
> Generally your CL is a significant improvement, so one circular dependency is
> probably fine. lgtm and thank you for doing this!

sigh, sorry for the mess. I removed the circular dependency with
ios/web/public/test_support, by moving all http_server code (http_server* files
and response providers) into one folder (http_server), since these two are
tightly coupled together. This led to a lot more mechanical changes...

I also factored out some test_constants in ios/web:test_support to have less
circular references.

Currently down to one, which I think is reasonable for now.

Again, sorry for all the churn, PTAL!

[Eugene But (OOO till 7-30), 2017-05-26 17:22:29]

Still lgtm. Thank you for doing this!

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/met...
File ios/chrome/browser/metrics/tab_usage_recorder_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/met...
ios/chrome/browser/metrics/tab_usage_recorder_egtest.mm:35: #import
"ios/web/public/test/http_server/http_server_util.h"
nit: s/import/include

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/net...
File ios/chrome/browser/net/cookies_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/net...
ios/chrome/browser/net/cookies_egtest.mm:19: #include
"ios/web/public/test/http_server/http_server.h"
nit: s/include/import

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File
ios/chrome/browser/ui/activity_services/activity_service_controller_egtest.mm
(right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/activity_services/activity_service_controller_egtest.mm:21:
#import "ios/web/public/test/http_server/http_server_util.h"
s/import/include

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/dialogs/javascript_dialog_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/dialogs/javascript_dialog_egtest.mm:25: #import
"ios/web/public/test/http_server/http_server_util.h"
s/import/include

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/find_bar/find_in_page_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/find_bar/find_in_page_egtest.mm:22: #import
"ios/web/public/test/http_server/http_server_util.h"
s/import/include

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/fullscreen_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/fullscreen_egtest.mm:27: #import
"ios/web/public/test/http_server/http_server_util.h"
s/import/include

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/print/print_controller_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/print/print_controller_egtest.mm:16: #import
"ios/web/public/test/http_server/http_server_util.h"
s/import/include

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm
(right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm:31:
#include "ios/web/public/test/http_server/http_server.h"
s/include/import

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/reading_list/reading_list_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/reading_list/reading_list_egtest.mm:37: #import
"ios/web/public/test/http_server/http_server_util.h"
s/import/include

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/sync/sync_fake_server_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/sync/sync_fake_server_egtest.mm:36: #import
"ios/web/public/test/http_server/http_server_util.h"
s/import/include

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/web...
File ios/chrome/browser/web/forms_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/web...
ios/chrome/browser/web/forms_egtest.mm:22: #import
"ios/web/public/test/http_server/http_server_util.h"
s/import/include

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/web...
File ios/chrome/browser/web/js_print_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/web...
ios/chrome/browser/web/js_print_egtest.mm:13: #import
"ios/web/public/test/http_server/http_server_util.h"
s/import/include

https://codereview.chromium.org/2898733003/diff/120001/ios/web/public/test/ht...
File ios/web/public/test/http_server/http_server.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/web/public/test/ht...
ios/web/public/test/http_server/http_server.mm:175:
std::string(base::IntToString(static_cast<int>(GetPort())));
There is no need to std::string. base::IntToString already returns std::string

https://codereview.chromium.org/2898733003/diff/120001/ios/web/public/test/ht...
File ios/web/public/test/http_server/response_provider.h (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/web/public/test/ht...
ios/web/public/test/http_server/response_provider.h:78: virtual
~ResponseProvider(){};
Dropping |;| will fix the formatting

[baxley, 2017-05-26 18:06:21]

Thank you for the thorough review!

I'm OOO this afternoon, so I'll land it next week, so I can make sure nothing
breaks downstream.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/met...
File ios/chrome/browser/metrics/tab_usage_recorder_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/met...
ios/chrome/browser/metrics/tab_usage_recorder_egtest.mm:35: #import
"ios/web/public/test/http_server/http_server_util.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> nit: s/import/include 

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/net...
File ios/chrome/browser/net/cookies_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/net...
ios/chrome/browser/net/cookies_egtest.mm:19: #include
"ios/web/public/test/http_server/http_server.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> nit: s/include/import

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File
ios/chrome/browser/ui/activity_services/activity_service_controller_egtest.mm
(right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/activity_services/activity_service_controller_egtest.mm:21:
#import "ios/web/public/test/http_server/http_server_util.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/dialogs/javascript_dialog_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/dialogs/javascript_dialog_egtest.mm:25: #import
"ios/web/public/test/http_server/http_server_util.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/find_bar/find_in_page_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/find_bar/find_in_page_egtest.mm:22: #import
"ios/web/public/test/http_server/http_server_util.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/fullscreen_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/fullscreen_egtest.mm:27: #import
"ios/web/public/test/http_server/http_server_util.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/print/print_controller_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/print/print_controller_egtest.mm:16: #import
"ios/web/public/test/http_server/http_server_util.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm
(right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm:31:
#include "ios/web/public/test/http_server/http_server.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> s/include/import

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/reading_list/reading_list_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/reading_list/reading_list_egtest.mm:37: #import
"ios/web/public/test/http_server/http_server_util.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/sync/sync_fake_server_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/sync/sync_fake_server_egtest.mm:36: #import
"ios/web/public/test/http_server/http_server_util.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/web...
File ios/chrome/browser/web/forms_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/web...
ios/chrome/browser/web/forms_egtest.mm:22: #import
"ios/web/public/test/http_server/http_server_util.h"
On 2017/05/26 17:22:28, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/web...
File ios/chrome/browser/web/js_print_egtest.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/chrome/browser/web...
ios/chrome/browser/web/js_print_egtest.mm:13: #import
"ios/web/public/test/http_server/http_server_util.h"
On 2017/05/26 17:22:29, Eugene But wrote:
> s/import/include

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/web/public/test/ht...
File ios/web/public/test/http_server/http_server.mm (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/web/public/test/ht...
ios/web/public/test/http_server/http_server.mm:175:
std::string(base::IntToString(static_cast<int>(GetPort())));
On 2017/05/26 17:22:29, Eugene But wrote:
> There is no need to std::string. base::IntToString already returns std::string

Done.

https://codereview.chromium.org/2898733003/diff/120001/ios/web/public/test/ht...
File ios/web/public/test/http_server/response_provider.h (right):

https://codereview.chromium.org/2898733003/diff/120001/ios/web/public/test/ht...
ios/web/public/test/http_server/response_provider.h:78: virtual
~ResponseProvider(){};
On 2017/05/26 17:22:29, Eugene But wrote:
> Dropping |;| will fix the formatting

Done.

[baxley, 2017-05-30 18:27:48]

The CQ bit was checked by baxley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-30 18:28:15]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-30 20:37:27]

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

[commit-bot: I haz the power, 2017-05-30 20:37:28]

Dry run: This issue passed the CQ dry run.

[baxley, 2017-05-30 21:19:02]

The CQ bit was checked by baxley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-30 21:19:50]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[baxley, 2017-05-30 22:41:25]

I uploaded a new patch with a workaround to not break downstream. Should have
just done this from the start. I'm doing a dry run, and it looks to be running
okay on a downstream tryjob. PTAL!

The changes I made were...
1. added back in //ios/web/test_support, which simply exposes the new
public/test utilities as public_deps.
2. added back in ios/web/public/test/http_server[_util].h, which just
include/import the header in ios/web/public/test/http_server/

[commit-bot: I haz the power, 2017-05-30 22:55:01]

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

[commit-bot: I haz the power, 2017-05-30 22:55:02]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Eugene But (OOO till 7-30), 2017-05-31 16:14:03]

still lgtm!

[baxley, 2017-05-31 16:29:59]

The CQ bit was checked by baxley@chromium.org

[commit-bot: I haz the power, 2017-05-31 16:30:35]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-31 16:45:09]

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

[commit-bot: I haz the power, 2017-05-31 16:45:12]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[baxley, 2017-05-31 16:56:40]

Description was changed from

==========
Split up ios/web:test_support.

No test files are in ios/web, so this creates BUILD.gn files
for the subdirectories. Update clients outside of ios/web to
depend on ios/web/public/test/*, instead of ios/web/test.

BUG=684616
==========

to

==========
Split up ios/web:test_support.

No test files are in ios/web, so this creates BUILD.gn files
for the subdirectories. Update clients outside of ios/web to
depend on ios/web/public/test/*, instead of ios/web/test.

BUG=684616
TBR=msarda,droger
==========

[baxley, 2017-05-31 16:56:40]

baxley@chromium.org changed reviewers:
+ droger@chromium.org, msarda@chromium.org

[baxley, 2017-05-31 16:59:30]

TBR msarda for change to components/signin/ios/browser/BUILD.gn
TBR droger for change to components/translate/ios/browser/BUILD.gn

Both of these are changes where ios/web:test_support is removed, and we're using
the more specific targets which were created in this CL.

[baxley, 2017-05-31 16:59:42]

The CQ bit was checked by baxley@chromium.org

[commit-bot: I haz the power, 2017-05-31 17:00:06]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-31 18:13:35]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1496249982068690,
"parent_rev": "873bac9ff01ac2c4969101a7d32408e10ce41ca2", "commit_rev":
"66cc96f0414e2c1b91d177e33314a7fa1008d644"}

[commit-bot: I haz the power, 2017-05-31 18:13:57]

Description was changed from

==========
Split up ios/web:test_support.

No test files are in ios/web, so this creates BUILD.gn files
for the subdirectories. Update clients outside of ios/web to
depend on ios/web/public/test/*, instead of ios/web/test.

BUG=684616
TBR=msarda,droger
==========

to

==========
Split up ios/web:test_support.

No test files are in ios/web, so this creates BUILD.gn files
for the subdirectories. Update clients outside of ios/web to
depend on ios/web/public/test/*, instead of ios/web/test.

BUG=684616
TBR=msarda,droger

Review-Url: https://codereview.chromium.org/2898733003
Cr-Commit-Position: refs/heads/master@{#475962}
Committed:
https://chromium.googlesource.com/chromium/src/+/66cc96f0414e2c1b91d177e33314...
==========

[commit-bot: I haz the power, 2017-05-31 18:14:00]

Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/66cc96f0414e2c1b91d177e33314...

[droger, 2017-06-07 09:25:05]

lgtm