Don't run chrome_frame_net_tests URLRequestTestHTTP over loopback, due to mysterious failures.

Don't run chrome_frame_net_tests URLRequestTestHTTP over loopback, due to mysterious failures.

Depends on:
http://codereview.chromium.org/9419053/ (make GetNetworkList filter-out 'down' adapters)
http://codereview.chromium.org/9368031/ (make HTTP tests parameterizable)
http://codereview.chromium.org/9369029/ (make TestServer parameterizable)

BUG=114369
TEST=chrome_frame_net_tests pass on the try bots.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=123118

[eroman, 2012-02-16 02:35:14]

http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_exte...
File chrome_frame/test/net/fake_external_tab.cc (right):

http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_exte...
chrome_frame/test/net/fake_external_tab.cc:583: struct hostent *host_info =
NULL;
style-nit: asterisk goes on left.

http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_exte...
chrome_frame/test/net/fake_external_tab.cc:585: LOG(ERROR) << "Calling
gethostname.";
Please only log to LOG(ERROR) if an error actually occured. LOG(INFO) would be
more appropriate here.

http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_exte...
chrome_frame/test/net/fake_external_tab.cc:588: << "over the loopback adapter,
which may result in hangs.";
nit: indent by one more.

http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_exte...
chrome_frame/test/net/fake_external_tab.cc:595: LOG(ERROR) << "Failed to look up
addresses for the local host name. Tests "
line longer than 80

http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_exte...
chrome_frame/test/net/fake_external_tab.cc:600:
inet_ntoa(*reinterpret_cast<in_addr *>(host_info->h_addr_list[0])));
Are you able to use the net code here? If you are, there are some utility
functions which might help in net_util.h:

net::GetHostName()
net::AddressToString()

http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_exte...
chrome_frame/test/net/fake_external_tab.cc:602: LOG(ERROR) <<
"chrome_frame_net_tests URLRequestTestHTTP will execute over "
long line.

[eroman, 2012-02-16 02:35:41]

oh oops, i published for the wrong CL!
Looking at the right one now.

[erikwright (departed), 2012-02-16 02:38:53]

I think you already figured it out, but this CL is not meant to be commit
or review ready yet. Sorry for the confusion and wasting your time.
On Feb 15, 2012 9:35 PM, <eroman@chromium.org> wrote:

>
> http://codereview.chromium.**org/9401013/diff/1/chrome_**
>
frame/test/net/fake_external_**tab.cc<http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_external_tab.cc>
> File chrome_frame/test/net/fake_**external_tab.cc (right):
>
> http://codereview.chromium.**org/9401013/diff/1/chrome_**
>
frame/test/net/fake_external_**tab.cc#newcode583<http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_external_tab.cc#newcode583>
> chrome_frame/test/net/fake_**external_tab.cc:583: struct hostent
> *host_info = NULL;
> style-nit: asterisk goes on left.
>
> http://codereview.chromium.**org/9401013/diff/1/chrome_**
>
frame/test/net/fake_external_**tab.cc#newcode585<http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_external_tab.cc#newcode585>
> chrome_frame/test/net/fake_**external_tab.cc:585: LOG(ERROR) << "Calling
> gethostname.";
> Please only log to LOG(ERROR) if an error actually occured. LOG(INFO)
> would be more appropriate here.
>
> http://codereview.chromium.**org/9401013/diff/1/chrome_**
>
frame/test/net/fake_external_**tab.cc#newcode588<http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_external_tab.cc#newcode588>
> chrome_frame/test/net/fake_**external_tab.cc:588: << "over the loopback
> adapter, which may result in hangs.";
> nit: indent by one more.
>
> http://codereview.chromium.**org/9401013/diff/1/chrome_**
>
frame/test/net/fake_external_**tab.cc#newcode595<http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_external_tab.cc#newcode595>
> chrome_frame/test/net/fake_**external_tab.cc:595: LOG(ERROR) << "Failed to
> look up addresses for the local host name. Tests "
> line longer than 80
>
> http://codereview.chromium.**org/9401013/diff/1/chrome_**
>
frame/test/net/fake_external_**tab.cc#newcode600<http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_external_tab.cc#newcode600>
> chrome_frame/test/net/fake_**external_tab.cc:600:
> inet_ntoa(*reinterpret_cast<**in_addr *>(host_info->h_addr_list[0]))**);
> Are you able to use the net code here? If you are, there are some
> utility functions which might help in net_util.h:
>
> net::GetHostName()
> net::AddressToString()
>
> http://codereview.chromium.**org/9401013/diff/1/chrome_**
>
frame/test/net/fake_external_**tab.cc#newcode602<http://codereview.chromium.org/9401013/diff/1/chrome_frame/test/net/fake_external_tab.cc#newcode602>
> chrome_frame/test/net/fake_**external_tab.cc:602: LOG(ERROR) <<
> "chrome_frame_net_tests URLRequestTestHTTP will execute over "
> long line.
>
>
http://codereview.chromium.**org/9401013/<http://codereview.chromium.org/9401...
>

[erikwright (departed), 2012-02-17 16:44:41]

Hi Robert, PTAL.

Eric, thanks for the initial feedback (I found GetNetworkList because of your
suggestion to look at the net library). More feedback welcome if you like.

Thanks,

Erik

[robertshield, 2012-02-17 17:40:54]

LGTM

http://codereview.chromium.org/9401013/diff/8001/chrome_frame/test/net/fake_e...
File chrome_frame/test/net/fake_external_tab.cc (right):

http://codereview.chromium.org/9401013/diff/8001/chrome_frame/test/net/fake_e...
chrome_frame/test/net/fake_external_tab.cc:633: for (size_t i = 0; i <
nic_list.size(); ++i) {
Consider adding a comment that we intentionally pick the first adapter found
with an address.

[eroman, 2012-02-17 21:55:31]

lgtm

http://codereview.chromium.org/9401013/diff/10001/chrome_frame/test/net/fake_...
File chrome_frame/test/net/fake_external_tab.h (right):

http://codereview.chromium.org/9401013/diff/10001/chrome_frame/test/net/fake_...
chrome_frame/test/net/fake_external_tab.h:142: void OverrideHttpHost();
[optional] nit: OverrideHttpServerHost.

[commit-bot: I haz the power, 2012-02-22 18:21:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/erikwright@chromium.org/9401013/10001

[commit-bot: I haz the power, 2012-02-22 19:22:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/erikwright@chromium.org/9401013/15001

[commit-bot: I haz the power, 2012-02-22 23:03:25]

Change committed as 123118