Stop using third_party\python_26 for many tests.

Stop using third_party\python_26 for many tests.

Migrate testserver.py from using embedded python in third_party\python_26 to
the python version installed on the system.

Kill many python_26 references.

It reduces the amount of data that needs to be transfered for Windows swarm
slaves.

The remaining uses of third_party\python_26 are:
1. chrome_frame.
2. pyauto needs the python SDK. As such, it needs a "known" python installation.
3. gn.

R=phajdan.jr@chromium.org,rsleevi@chromium.org
BUG=98636
BUG=321703
TEST=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238989

[Paweł Hajdan Jr., 2013-11-15 19:34:42]

+bratell FYI

https://codereview.chromium.org/74253002/diff/1/net/test/python_utils.cc
File net/test/python_utils.cc (right):

https://codereview.chromium.org/74253002/diff/1/net/test/python_utils.cc#newc...
net/test/python_utils.cc:112:
//python_cmd->SetProgram(base::FilePath(FILE_PATH_LITERAL("python")));
Remove the commented-out line.

https://codereview.chromium.org/74253002/diff/1/net/test/python_utils.cc#newc...
net/test/python_utils.cc:118: #elif defined(OS_POSIX)
This pre-processor conditional is unterminated, please add #endif.

https://codereview.chromium.org/74253002/diff/1/net/test/python_utils.cc#newc...
net/test/python_utils.cc:122: python_cmd->AppendArg("-u");
Why don't we do the same for Windows?

https://codereview.chromium.org/74253002/diff/1/net/test/spawned_test_server/...
File net/test/spawned_test_server/local_test_server_win.cc (left):

https://codereview.chromium.org/74253002/diff/1/net/test/spawned_test_server/...
net/test/spawned_test_server/local_test_server_win.cc:201: ScopedPath
python_path(python_dir);
Yay, looks like now we no longer need ScopedPath. Let's remove it.

https://codereview.chromium.org/74253002/diff/1/net/tools/testserver/testserv...
File net/tools/testserver/testserver.py (right):

https://codereview.chromium.org/74253002/diff/1/net/tools/testserver/testserv...
net/tools/testserver/testserver.py:44: # Append these paths at the the end of
sys.path so the system lib will be used by default.
nit: 80 chars

https://codereview.chromium.org/74253002/diff/1/net/tools/testserver/testserv...
net/tools/testserver/testserver.py:46: sys.path.append(os.path.join(ROOT_DIR,
'third_party', 'tlslite'))
The tests don't work with system tlslite. We depend on our custom modifications.
Please make it continue to use bundled tlslite.

https://codereview.chromium.org/74253002/diff/1/net/tools/testserver/testserv...
net/tools/testserver/testserver.py:54: BASE_DIR =
os.path.dirname(os.path.abspath(__file__))
BASE_DIR is already defined here...

[M-A Ruel, 2013-11-15 19:40:26]

It's not ready for review yet. I'm uploading so I can test it on the TS. For
example, see patchset #2.

[M-A Ruel, 2013-11-15 21:51:14]

Ready for review except for one net_unittests test case that needs to be fixed
on Windows;
[ RUN      ] ProxyScriptFetcherImplTest.NoCache

HTTP server started on 127.0.0.1:49428...
sending server_data: {"host": "127.0.0.1", "port": 49428} (36 bytes)
e:\b\build\slave\win\build\src\net\proxy\proxy_script_fetcher_impl_unittest.cc(334):
error: Expected: (OK) != (callback.WaitForResult()), actual: 0 vs 0

[  FAILED  ] ProxyScriptFetcherImplTest.NoCache (686 ms)

[1771/7185] ProxyScriptFetcherImplTest.NoCache (686 ms)

[Paweł Hajdan Jr., 2013-11-16 00:13:50]

https://codereview.chromium.org/74253002/diff/100002/net/test/python_utils.cc
File net/test/python_utils.cc (right):

https://codereview.chromium.org/74253002/diff/100002/net/test/python_utils.cc...
net/test/python_utils.cc:113:
python_cmd->SetProgram(base::FilePath(FILE_PATH_LITERAL("cmd.exe")));
Why do we need to go through cmd.exe /c ?

https://codereview.chromium.org/74253002/diff/100002/net/test/python_utils.cc...
net/test/python_utils.cc:116: 
nit: Remove empty line.

[Daniel Bratell, 2013-11-19 10:23:13]

Just curious: Have you considered relying on the machine having a good enough
python in the path? I guess it's not perfect and will have problems of its own
but I'd like to know if there is anything specific that blocks. The Python SDK
you mentioned in pyauto?

[M-A Ruel, 2013-11-19 14:20:03]

On 2013/11/19 10:23:13, Daniel Bratell wrote:
> Just curious: Have you considered relying on the machine having a good enough
> python in the path? I guess it's not perfect and will have problems of its own
> but I'd like to know if there is anything specific that blocks. The Python SDK
> you mentioned in pyauto?

That's exactly what this CL is doing by removing reliance on the embedded
version. The blocking things are listed in the CL description, but everything
except gn is going away, gn will simply need to have its config files fixed.

[M-A Ruel, 2013-11-20 17:43:45]

I figured out why ProxyScriptFetcherImplTest.NoCache fails. It is because
LocalTestServer::Stop() kills the child process but in that case it is cmd.exe,
and it doesn't properly kill the dependent process.

Some options:
- Create a windows version that kill the process tree. I have an old version of
code supporting that that I am reviving.
- Tweak it on windows to properly kill the child process of cmd.exe.
- Use a pipe/event/url to tell the server to kill itself.

[Paweł Hajdan Jr., 2013-11-20 18:23:38]

On 2013/11/20 17:43:45, M-A Ruel wrote:
> I figured out why ProxyScriptFetcherImplTest.NoCache fails. It is because
> LocalTestServer::Stop() kills the child process but in that case it is
cmd.exe,
> and it doesn't properly kill the dependent process.

One thing that surprises me is we use job objects to kill the whole process
tree. Maybe for some reason that doesn't work, or maybe we aren't freeing the
job object early enough.

> Some options:
> - Create a windows version that kill the process tree. I have an old version
of
> code supporting that that I am reviving.

That would be fine for me.

> - Tweak it on windows to properly kill the child process of cmd.exe.

Sounds suspiciously hacky, but may be fine depending on what it means.

> - Use a pipe/event/url to tell the server to kill itself.

That's how it worked in the beginning. It's unreliable when the server hangs or
otherwise misbehaves, which can happen.

[M-A Ruel, 2013-11-20 18:56:45]

+Ryan for the NoCache problem.

[Ryan Sleevi, 2013-11-25 07:09:23]

Adding rvargas here. I don't have much to add on creative solutions. Ricardo
probably knows a way to make sure the job object kills the tree - or the gotchas
on why it doesn't work.

[rvargas (doing something else), 2013-11-25 20:07:34]

On 2013/11/25 07:09:23, Ryan Sleevi wrote:
> Adding rvargas here. I don't have much to add on creative solutions. Ricardo
> probably knows a way to make sure the job object kills the tree - or the
gotchas
> on why it doesn't work.

I'm totally unfamiliar with any job objects created around the python server...
what I see here (having an extra cmd.exe between the caller and the server) fits
perfectly with M-A explanation of now killing the wrong process. From that point
of view, either one of the two first options make sense to me (adjusting Stop to
kill the right process).

Sorry for not being helpful here.

[M-A Ruel, 2013-11-30 02:05:34]

On 2013/11/25 20:07:34, rvargas wrote:
> On 2013/11/25 07:09:23, Ryan Sleevi wrote:
> > Adding rvargas here. I don't have much to add on creative solutions. Ricardo
> > probably knows a way to make sure the job object kills the tree - or the
> gotchas
> > on why it doesn't work.
> 
> I'm totally unfamiliar with any job objects created around the python
server...
> what I see here (having an extra cmd.exe between the caller and the server)
fits
> perfectly with M-A explanation of now killing the wrong process. From that
point
> of view, either one of the two first options make sense to me (adjusting Stop
to
> kill the right process).

Ugh, the job object was never closed, so the process was not terminated. The fix
was simply to close the job handle. See new patchset. There's no need to disable
any test now.

[Paweł Hajdan Jr., 2013-12-02 19:37:55]

LGTM with comments.

https://codereview.chromium.org/74253002/diff/410001/net/test/python_utils.cc
File net/test/python_utils.cc (right):

https://codereview.chromium.org/74253002/diff/410001/net/test/python_utils.cc...
net/test/python_utils.cc:117: 
nit: Remove empty line.

https://codereview.chromium.org/74253002/diff/410001/net/test/spawned_test_se...
File net/test/spawned_test_server/local_test_server.cc (right):

https://codereview.chromium.org/74253002/diff/410001/net/test/spawned_test_se...
net/test/spawned_test_server/local_test_server.cc:130: #if defined(OS_WIN)
Let's close the job handle always when returning from this method, not just when
WaitForSingleProcess fails.

[M-A Ruel, 2013-12-02 20:13:15]

https://codereview.chromium.org/74253002/diff/410001/net/test/python_utils.cc
File net/test/python_utils.cc (right):

https://codereview.chromium.org/74253002/diff/410001/net/test/python_utils.cc...
net/test/python_utils.cc:117: 
On 2013/12/02 19:37:56, Paweł Hajdan Jr. wrote:
> nit: Remove empty line.

Done.

https://codereview.chromium.org/74253002/diff/410001/net/test/spawned_test_se...
File net/test/spawned_test_server/local_test_server.cc (right):

https://codereview.chromium.org/74253002/diff/410001/net/test/spawned_test_se...
net/test/spawned_test_server/local_test_server.cc:130: #if defined(OS_WIN)
On 2013/12/02 19:37:56, Paweł Hajdan Jr. wrote:
> Let's close the job handle always when returning from this method, not just
when
> WaitForSingleProcess fails.

and removed stale comment at lines 136-137.

[commit-bot: I haz the power, 2013-12-02 20:14:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/74253002/430001

[commit-bot: I haz the power, 2013-12-03 09:28:24]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[commit-bot: I haz the power, 2013-12-03 13:54:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/74253002/430001

[commit-bot: I haz the power, 2013-12-03 15:14:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/74253002/440001

[M-A Ruel, 2013-12-03 16:14:10]

On 2013/12/03 15:14:55, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/maruel%40chromium.org/74253002/440001

net_unittests is still broken on android_dbg_triggered_tests.

http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...
--- CUT HERE ---
[ RUN      ] URLFetcherPostTest.Basic
I  342s Thread-56  Action for POST method is: /start.
I  342s Thread-56  Handling request to spawn a test server.
I  342s Thread-56  102
I  342s Thread-56 
{"data-dir":"chrome/test/data","host":"127.0.0.1","log-to-console":null,"port":0,"server-type":"http"}
I  342s Thread-175  Start running the thread!
I  342s Thread-175  Running:
['/b/build/slave/android/build/src/net/tools/testserver/testserver.py',
'--port=0', '--startup-pipe=16', u'--host=127.0.0.1',
u'--data-dir=/b/build/slave/android/build/src/chrome/test/data',
'--log-to-console']
E  344s Thread-173  Failed to wait to the Python test server to be started.
E  344s Thread-173  Trying to unmap non-forwarded port 0
I  344s Thread-173  Test-server has died.
localhost - - [03/Dec/2013 08:12:13] "POST /start HTTP/1.1" 500 -
I  344s Thread-68  Encounter problem during starting a test server.
[ERROR:spawner_communicator.cc(299)] Spawner server returned bad status:
HTTP/1.0 500 Test Server Error.
[ERROR:spawner_communicator.cc(241)] request failed, status: 2, error: -3
../../net/url_request/url_fetcher_impl_unittest.cc:1107: Failure
Value of: test_server.Start()
  Actual: false
Expected: true
I  344s Thread-68  Action for GET method is: /kill.
[ERROR:spawner_communicator.cc(241)] request failed, status: 3, error: -324
[  FAILED  ] URLFetcherDownloadProgressTest.Basic (10113 ms)
--- CUT HERE ---

It seems I broke something there but I don't know what exactly and it's not easy
for me to test at the moment. :/

[commit-bot: I haz the power, 2013-12-03 19:10:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/74253002/440001

[commit-bot: I haz the power, 2013-12-03 19:29:44]

Failed to trigger a try job on android_dbg
HTTP Error 400: Bad Request

[M-A Ruel, 2013-12-04 21:16:42]

+bulach for build/android/pylib/chrome_test_server_spawner.py

https://codereview.chromium.org/74253002/diff/480001/build/android/pylib/chro...
File build/android/pylib/chrome_test_server_spawner.py (left):

https://codereview.chromium.org/74253002/diff/480001/build/android/pylib/chro...
build/android/pylib/chrome_test_server_spawner.py:28: # Path that are needed to
import necessary modules when launching a testserver.
Modifying os.environ as a side-effect of importing a module is not what can be
called "good practices".

It should not be necessary anyway, android is not different from other platforms
with that regard.

[M-A Ruel, 2013-12-05 15:08:41]

On 2013/12/04 21:16:42, M-A Ruel wrote:
> +bulach for build/android/pylib/chrome_test_server_spawner.py
> 
>
https://codereview.chromium.org/74253002/diff/480001/build/android/pylib/chro...
> File build/android/pylib/chrome_test_server_spawner.py (left):
> 
>
https://codereview.chromium.org/74253002/diff/480001/build/android/pylib/chro...
> build/android/pylib/chrome_test_server_spawner.py:28: # Path that are needed
to
> import necessary modules when launching a testserver.
> Modifying os.environ as a side-effect of importing a module is not what can be
> called "good practices".
> 
> It should not be necessary anyway, android is not different from other
platforms
> with that regard.

Reverted this file from this CL. This is not strictly needed here.

[commit-bot: I haz the power, 2013-12-05 15:09:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/74253002/500001

[commit-bot: I haz the power, 2013-12-05 17:50:14]

Change committed as 238989

[bulach, 2013-12-06 09:56:53]

clarification about the android file in the old patchset:

https://codereview.chromium.org/74253002/diff/480001/build/android/pylib/chro...
File build/android/pylib/chrome_test_server_spawner.py (left):

https://codereview.chromium.org/74253002/diff/480001/build/android/pylib/chro...
build/android/pylib/chrome_test_server_spawner.py:28: # Path that are needed to
import necessary modules when launching a testserver.
On 2013/12/04 21:16:43, M-A Ruel wrote:
> Modifying os.environ as a side-effect of importing a module is not what can be
> called "good practices".
> 
> It should not be necessary anyway, android is not different from other
platforms
> with that regard.

sorry about the delay, pliard@ would be a better reviewer for this file..
afaict, the problem is that the top-level test harness for android all start in
build/android, and then some specific tests need to start these servers..
I think this is hooked up at a different level for other platforms.