Automatic password manager tests now rely on being killed by the bot on timeout

Automatic password manager tests now rely on being killed by
the bot on timeout

Replace Linux specific commands that were used before by taking advantage of the test bots behavior that stop the
tests if no output was displayed for a few minutes

BUG=393531

[rchtara, 2014-07-21 08:27:59]

Hi
This is the new cl.
Could you please review it.
Thanks a lot
Riadh

[vabr (Chromium), 2014-07-21 08:39:52]

Thanks, Riadh!

2 comments:
(1) Please improve the CL title by following these steps:
   (1a) Make sure to mention the password manager (there are too many "tests" in
Chromium).
   (1b) Make sure to mention the actual change ("Automatic password manager
tests now rely on being killed by the bot on timeout").
   (1c) Once you do the changes, make sure that the first line of the CL
description is the same as the title, AND the second line is blank.

(2) Comment in the code (see below).

Thanks for your hard work!
Vaclav

https://codereview.chromium.org/403323002/diff/20001/components/test/data/pas...
File components/test/data/password_manager/run_tests.py (right):

https://codereview.chromium.org/403323002/diff/20001/components/test/data/pas...
components/test/data/password_manager/run_tests.py:89:
tests.saveResults(tests_results, results_path)
Why would you save the tests to a file, and immediately read them back?
Could you now get rid of the results file altogether?

[rchtara, 2014-07-21 15:12:41]

Could you check the cl again
Thanks a lot
Riadh

https://codereview.chromium.org/403323002/diff/20001/components/test/data/pas...
File components/test/data/password_manager/run_tests.py (right):

https://codereview.chromium.org/403323002/diff/20001/components/test/data/pas...
components/test/data/password_manager/run_tests.py:89:
tests.saveResults(tests_results, results_path)
I wait for the answer of Aaron answer to know if it's possible to read  the json
from a file (The tests log has many things that are not in json format, and I
dont want to change that, so if someone wants to run the tests on it's own
machine, he should get a plain text log instead of json)
There is now two kind of result data:
*the first one is xml that holds all details about tests and it's going to be
send to appengine (not saved)
* the second is a very simple json file for the recipe (saved)

[rchtara, 2014-07-21 15:12:42]

Could you check the cl again
Thanks a lot
Riadh

[vabr (Chromium), 2014-07-22 07:54:28]

Thank you, Riadh.
I have some more comments below.
Cheers,
Vaclav

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
File components/test/data/password_manager/run_tests.py (right):

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:9: import urllib2
This also needs to be deleted if |response| and }request| are (see the comment
at the end).

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:47: print "Running tests for
%s " % websitetest.name
nit: Please add a comment explaining that this message needs to be printed out
to let the trybot know that the tests are still working, so they should not be
killed.

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:78: xml = xml +
tests.resultsToXml(tests_results)
xml += ...

(also below)

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:82: else:
The "else" branch scraps the test results, even if there is one. That might be
confusing: Imagine someone would suspect they might have broken the "prompt"
version, but they will see an empty failure for both.
Please make sure to report all results we got, and only enter dummy failure logs
for a particular test if we really did not get any results for it.

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:92: request =
urllib2.Request("http://dashbord", data=xml,
Please do not include code which is not working. Until the endpoint (dashboard)
is ready, the script should not send anything to it.
Please delete |request| and |response| for now. You might leave a comment saying
that |xml| should be sent to the dashboard, once it is ready.

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:95: json =
open(args.save_path[0], "w")
When I run a test in Rietveld, and want to see a result, I can always click on
the "stdio" link for every step (see, e.g.,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/15722)
to see the output. I understand that that's what the test outputs to its
standard output. Could you please confirm that, and if so, make sure to print
the information you also store in the XML format as plain text to stdout, so
that even without the dashboard we an already see the results?

[rchtara, 2014-07-22 12:49:36]

Hi
Could you please review this cl.
Thanks a lot
Riadh

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
File components/test/data/password_manager/run_tests.py (right):

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:9: import urllib2
On 2014/07/22 07:54:27, vabr (Chromium) wrote:
> This also needs to be deleted if |response| and }request| are (see the comment
> at the end).

Done.

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:47: print "Running tests for
%s " % websitetest.name
On 2014/07/22 07:54:28, vabr (Chromium) wrote:
> nit: Please add a comment explaining that this message needs to be printed out
> to let the trybot know that the tests are still working, so they should not be
> killed.

Done.

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:78: xml = xml +
tests.resultsToXml(tests_results)
On 2014/07/22 07:54:28, vabr (Chromium) wrote:
> xml += ...
> 
> (also below)

Done.

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:82: else:
On 2014/07/22 07:54:27, vabr (Chromium) wrote:
> The "else" branch scraps the test results, even if there is one. That might be
> confusing: Imagine someone would suspect they might have broken the "prompt"
> version, but they will see an empty failure for both.
> Please make sure to report all results we got, and only enter dummy failure
logs
> for a particular test if we really did not get any results for it.

Done.

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:92: request =
urllib2.Request("http://dashbord", data=xml,
On 2014/07/22 07:54:27, vabr (Chromium) wrote:
> Please do not include code which is not working. Until the endpoint
(dashboard)
> is ready, the script should not send anything to it.
> Please delete |request| and |response| for now. You might leave a comment
saying
> that |xml| should be sent to the dashboard, once it is ready.

Done.

https://codereview.chromium.org/403323002/diff/120001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:95: json =
open(args.save_path[0], "w")
Yes I confirm that.
The new patch allows to print the results to stdio.

[vabr (Chromium), 2014-07-22 15:41:46]

Thanks, Riadh.
LGTM, with the comments below addressed.
Cheers,
Vaclav

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
File components/test/data/password_manager/run_tests.py (right):

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:62: # This message needs to
be printed out to let the trybot know that the
typo: know -> knows

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:82: except Exception:
Could you print the textual description of the exception?
except Exception as e:
  print str(e)
(also below)

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:107: xml +=
tests.resultsToXml(tests_results)
You currently duplicate here the code from lines 127 and 128.
If you remove lines 107-109, and instead put the lines
if successful:
  break
after line 128, you remove that duplicity.

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:117: "An expected error has
occurred."))
expected -> unexpected?
(also below)

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:119: elif len(tests_results)
== 0::
typo: :: -> :

[rchtara, 2014-07-22 16:18:48]

Hello,
I created the dashboard with appengine (it only saves the xml data into a
spreadsheet.)
So you can see the dashboard part reappears.
Could you please have a look?
Thanks a lot
Riadh

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
File components/test/data/password_manager/run_tests.py (right):

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:62: # This message needs to
be printed out to let the trybot know that the
http://www.oxfordlearnersdictionaries.com/definition/english/let_1 (let
somebody/something do something)
:)

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:82: except Exception:
On 2014/07/22 15:41:45, vabr (Chromium) wrote:
> Could you print the textual description of the exception?
> except Exception as e:
>   print str(e)
> (also below)

Done.

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:107: xml +=
tests.resultsToXml(tests_results)
On 2014/07/22 15:41:45, vabr (Chromium) wrote:
> You currently duplicate here the code from lines 127 and 128.
> If you remove lines 107-109, and instead put the lines
> if successful:
>   break
> after line 128, you remove that duplicity.

Done.

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:117: "An expected error has
occurred."))
On 2014/07/22 15:41:45, vabr (Chromium) wrote:
> expected -> unexpected?
> (also below)

Done.

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:119: elif len(tests_results)
== 0::
On 2014/07/22 15:41:45, vabr (Chromium) wrote:
> typo: :: -> :

Done.

[vabr (Chromium), 2014-07-22 16:32:04]

Thank you, Riadh.

I'm confused about the dashboard (see my comment below).
The rest of the code LGTM.

Cheers,
Vaclav

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
File components/test/data/password_manager/run_tests.py (right):

https://codereview.chromium.org/403323002/diff/200001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:62: # This message needs to
be printed out to let the trybot know that the
On 2014/07/22 16:18:47, rchtara wrote:
> http://www.oxfordlearnersdictionaries.com/definition/english/let_1 (let
> somebody/something do something)
> :)
> 

You are right, I missed the "let"! :)
Never mind, then.

https://codereview.chromium.org/403323002/diff/240001/components/test/data/pa...
File components/test/data/password_manager/run_tests.py (right):

https://codereview.chromium.org/403323002/diff/240001/components/test/data/pa...
components/test/data/password_manager/run_tests.py:136:
urllib2.urlopen("http://savvy-hull-649.appspot.com/?" + text).read()
Are you throwing out the response? If it can give you any diagnostics, please
consider printing that out.
Also, the dashboard should be on the internal Appengine (*.googleplex.com). Why
is it called savvy-hull-649, and how can the saved data be accessed?