938664 - Talos unit tests are broken

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[William Lachance (:wlach)]

Attachment: Log of talos unit tests failing

If I install talos locally, then run INSTALL.py, then call runtests.py with the installed python, the unit tests hang here:

test_cli (test_PerfConfigurator.PerfConfiguratorUnitTest) ...  - outputName = /tmp/tmpXd2K5R.yaml
ok
test_errors (test_PerfConfigurator.PerfConfiguratorUnitTest) ...  - outputName = /tmp/tmpKFde6G.yaml

If I press CTRL-C, they continue on for a bit, before failing on an undefined import.

(see attached file for full log)

In addition to fixing tis issue, we should (1) make running unit tests a mandatory check before committing code and (2) run them continuously.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

test_ps.py is the main culprit here- a lot of this code was removed when switching to mozprocess- let me update to use mozprocess calls.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: fix the test_ps.py for talos (1.0)

this really just removes the test case which tested functions that were replaced by mozprocess.

Comment 3

[William Lachance (:wlach)]

Comment on attachment 8333816 [details] [diff] [review]
fix the test_ps.py for talos (1.0)

Review of attachment 8333816 [details] [diff] [review]:
-----------------------------------------------------------------

At this point it looks more like you're just retesting mozprocess's ps function, which I assume we already do inside mozbase itself. I would suggest just killing test_ps.py altogether.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: remove test_ps.py from talos (2.0)

ok, removing it is easy.

Comment 5

[William Lachance (:wlach)]

Comment on attachment 8333958 [details] [diff] [review]
remove test_ps.py from talos (2.0)

Great. Please commit.

Do the unit tests now pass for you with no issue? As mentioned above, I saw some hangs before with the PerfConfigurator tests...

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

it takes about 30 seconds to complete that test, but it passes as ok

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

https://hg.mozilla.org/build/talos/rev/ad927d1aa867

Comment 8

[William Lachance (:wlach)]

I am still seeing errors.

Comment 9

[William Lachance (:wlach)]

Attachment: Error log after patch was applied

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: fix test to do str(e) since we don't have e.msg anymore (1.0)

fix an error that shows up when running in the virtualenv.  There is still an issue with test_PerfConfigurator.py as we attempt to do a remote perf configurator on device 0.0.0.0 and this yields an exception (which we are testing for).  On some machines this doesn't timeout and causes a hang/error.

Comment 11

[William Lachance (:wlach)]

Comment on attachment 8335366 [details] [diff] [review]
fix test to do str(e) since we don't have e.msg anymore (1.0)

Hmm, I didn't think this was necessary, but apparently it is:

>>> try:
...     raise Exception("this is a string")
... except Exception, e:
...     print "this is" in e
...     print "this is not" in e
... 
False
False
>>> try:
...     raise Exception("this is a string")
... except Exception, e:
...     print "this is" in str(e)
...     print "this is not" in str(e)
... 
True
False

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

https://hg.mozilla.org/build/talos/rev/610eda814385

Comment 13

[William Lachance (:wlach)]

(In reply to Joel Maher (:jmaher) from comment #10)
> Created attachment 8335366 [details] [diff] [review]
> fix test to do str(e) since we don't have e.msg anymore (1.0)
> 
> fix an error that shows up when running in the virtualenv.  There is still
> an issue with test_PerfConfigurator.py as we attempt to do a remote perf
> configurator on device 0.0.0.0 and this yields an exception (which we are
> testing for).  On some machines this doesn't timeout and causes a hang/error.

Ok, did some investigation. I think it's not so much that it isn't timing out, but that the timeout takes FOREVER, since we retry a whole bunch of times trying to get the device root on initialization (5 times, over an increasing number of seconds). 

There's also a problem with test_install where the virtualenv command returns non-zero because virtualenv_support isn't installed. I think this is a new requirement in virtualenv, and since we're just downloading the copy of virtualenv from github master, we're seeing this problem.

I would propose that as an interim measure we disable these tests, then file followup bugs to re-enable them at some point in the future. Broken tests are worse than no tests.

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

there is no easy way to reduce the retries or timeouts- unless we instrument the code in depth, I am fine removing this one specific scenario from test_PerfConfigurator.py

Comment 15

[Julien Pagès (:parkouss)]

Is this bug still open ? I would like to work on that if it is the case.

(In reply to William Lachance (:wlach) from comment #0)

> In addition to fixing tis issue, we should (1) make running unit tests a
> mandatory check before committing code and (2) run them continuously.

How can we achieve this with Talos ?

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

this bug is still open.  We should remove the tests for remote and ensure everything else is running smoothly.

A couple of thoughts:
1) run tests on github with tox, coveralls, etc.
2) either move talos to github, or mirror it there where a new checkin on hg goes to github and runs tests
3) if we moved talos into mozilla-central, what type of issues might this cause?

Personally I would lean more towards github so we can submit a PR and get unittests, etc. run on it right away before even reviewing it.  There are a lot of other good reasons to move talos into the tree though, so maybe mirror to github until we solve the in-tree move, etc.

Comment 17

[William Lachance (:wlach)]

I agree 100%, if we can get a github<->hg mirror going I think a github repo is the way to go. That way we can get all this going, and the only thing we'd have to change would be the location of the hg repo in talos.json.

IIRC Hal was the person to talk to about these things. Hal, how difficult would this be to set up? We already have a build-talos repository on github which mirrors from http://hg.mozilla.org/build/talos -- how hard would it be to create a *new* hg repository which mirrors from a new github repository? (we'd probably want to keep http://hg.mozilla.org/build/talos around as-is for a while for backwards compatibility with older versions of the tree, etc)

Comment 18

[William Lachance (:wlach)]

Looks like releng is doing something similar to this proposal with some of their repositories: http://coop.deadsquid.com/2015/03/better-releng-patch-contribution-workflow/

Comment 19

[Hal Wine [:hwine] use NI!]

(In reply to William Lachance (:wlach) from comment #18)
> Looks like releng is doing something similar to this proposal with some of
> their repositories:
> http://coop.deadsquid.com/2015/03/better-releng-patch-contribution-workflow/

So, releng is going through some of that pain because they started on hg. Our criteria of where the RoR depends on how the code is deployed, and where the code is deployed, in the CI environment. Code that is auto deployed to build systems must have RoR on *.mozilla.org

If this code is only for test, what's the value of an hg.mozilla.org repo for the new code?

So, yes, it can be mirrored from github to hg.mozilla.org, but I would recommend that only if you have a real need. If so, file a request per https://wiki.mozilla.org/ReleaseEngineering/RepositoryCreationRequest#mirroring

Comment 20

[Joel Maher ( :jmaher ) (UTC -8)]

In production, we use hg.mozilla.org/build/talos to run all talos tests.  We clone from that repository for all talos runs.

The goal is to run tests as we do on github.  For the simpler solution, we could make github the canonical source, then mirror to the existing hg repo.

that seems like the best approach overall.

Comment 21

[William Lachance (:wlach)]

(In reply to Hal Wine [:hwine] (use needinfo) from comment #19)
> (In reply to William Lachance (:wlach) from comment #18

> If this code is only for test, what's the value of an hg.mozilla.org repo
> for the new code?

The only value is that we don't have to change the build automation, which assumes that talos will be cloned from an hg repository. It's not worth changing that if we're planning to move the whole thing into the tree at some point.

Comment 22

[Julien Pagès (:parkouss)]

The unit tests are passing now. But we should find a way to automate the unit tests runs.

Comment 23

[Julien Pagès (:parkouss)]

I'm closing this, because running unit tests with:

python setup.py test

run fine now. Please reopen the bug if I missed something.