605306 - [tracking] make all unittests pass

Status: RESOLVED FIXED

(Socorro :: General, task, P1)

Description

[Robert Helmer [:rhelmer]]

We are close to having all unittest pass on Hudson. I've examined the few remaining and I think they should be disabled (will have a patch for this momentarily).

Once all unittests pass, we can focus on improving code coverage and not breaking existing tests (unless they are broken and/or useless, in which case we should disable or remove them as needed).

Comment 1

[Robert Helmer [:rhelmer]]

Attachment: disable known-broken unittests

I've added FIXME comments in there about what the problems seem to be.

There appears to be only one test that needs an HBase server to test against (testHbaseResubmit.py), we made an exception for PostgreSQL since so many things need it and it's relatively easy to install, but I suggest we instead mock the thrift connection.

For what it's worth, I'd like to move the PostgreSQL connections to use mocks too but it's a sizable amount of work at this point, and having somewhat suboptimal tests is way better than having no tests.

Comment 2

[Robert Helmer [:rhelmer]]

Sending        socorro/unittest/cron/testDailyUrl.py
Sending        socorro/unittest/cron/testHbaseResubmit.py
Sending        socorro/unittest/lib/testStats.py
Transmitting file data ...
Committed revision 2653.

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Post-fix screenshot, showing tests passing in Hudson

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on https://hudson.mozilla.org/job/socorro/lastCompletedBuild/testReport/.

Comment 5

[Robert Helmer [:rhelmer]]

Regressed because 1.7reversion branch has been copied to trunk; I'll redo the patch against the new trunk.

Comment 6

[Robert Helmer [:rhelmer]]

(In reply to comment #5)
> Regressed because 1.7reversion branch has been copied to trunk; I'll redo the
> patch against the new trunk.

s/has been copied to/will be copied to/

There were some unexpected failures, all around the postgresql (non-mock) test:

CRITICAL: relation "productdims" does not exist

This might be some intermittent problem in the tests.

Comment 7

[Robert Helmer [:rhelmer]]

Attachment: [as landed] unittest fixes

unit tests fixes:
* set PYTHONPATH correctly
* only clobber unittest configs in hudson target
* call phpunit from coverage targets too

Comment 8

[Robert Helmer [:rhelmer]]

Attachment: simplify coverage, pass PYTHONPATH consistently

Comment 9

[Robert Helmer [:rhelmer]]

Attachment: simplify coverage, pass PYTHONPATH consistently

Updated since attachment 491277 [details] [diff] [review] landed already.

Comment 10

[Robert Helmer [:rhelmer]]

Comment on attachment 491285 [details] [diff] [review]
simplify coverage, pass PYTHONPATH consistently

Thanks!

Sending        Makefile
Transmitting file data .
Committed revision 2727.

Comment 11

[Robert Helmer [:rhelmer]]

It looks like we have at least one intermittent test:
https://hudson.mozilla.org/job/socorro/lastCompletedBuild/testReport/socorro.unittest.processor.testProcessor/TestProcessor/testSubmitJobToThreads/

We've also seen it hang a few times, not sure if this is the unit test hanging or some problem in Hudson, but we can abort from Hudson so I am guessing it's the test getting into a loop or something:
https://hudson.mozilla.org/job/socorro/155/console
https://hudson.mozilla.org/job/socorro/158/console

Comment 12

[K Lars Lohn [:lars] [:klohn]]

I've seen unit tests hang when they are testing a multithreaded component and an assert fails.  The assertion exception gets raised and bounces execution beyond the multithreaded components cleanup/join code.  The solution is to make sure that all tests that involve multithreaded components use a try:finally: block to ensure that the cleanup/join code gets run after an assertion failure.

I've not yet looked to see if that is true in your reported cases...

Comment 13

[Robert Helmer [:rhelmer]]

Another hang in testProcessor:

https://hudson.mozilla.org/job/socorro/165/console

I'm going to disable that one, and run it in a loop locally to see if I can reproduce.

(In reply to comment #12)
> I've seen unit tests hang when they are testing a multithreaded component and
> an assert fails.  The assertion exception gets raised and bounces execution
> beyond the multithreaded components cleanup/join code.  The solution is to make
> sure that all tests that involve multithreaded components use a try:finally:
> block to ensure that the cleanup/join code gets run after an assertion failure.
> 
> I've not yet looked to see if that is true in your reported cases...

This does sound consistent with the behaviors we've seen (intermittent failure, and intermittent hang). I'll take a peek while I try to repro it locally.

Comment 14

[Robert Helmer [:rhelmer]]

(In reply to comment #13)
> Another hang in testProcessor:
> 
> https://hudson.mozilla.org/job/socorro/165/console

I take that back - it looks like it was actually socorro.unittest.lib.testJsonDumpStorage that was hanging.

Comment 15

[Robert Helmer [:rhelmer]]

(In reply to comment #14)
> (In reply to comment #13)
> > Another hang in testProcessor:
> > 
> > https://hudson.mozilla.org/job/socorro/165/console
> 
> I take that back - it looks like it was actually
> socorro.unittest.lib.testJsonDumpStorage that was hanging.

So the reason I am confused here is that when I looked while it was hanging, it appeared, "socorro.unittest.lib.testJsonDumpStorage" was the last test output we had.. looking at the console output after aborting, it looks more like it was socorro.unittest.processor.testProcessor, I am guessing this is due to stdout/stderr buffering by Hudson.

Anyway, I am going to assume the problem is socorro.unittest.processor.testProcessor for now, and focus on that for a while.

Comment 16

[Robert Helmer [:rhelmer]]

Here is one I haven't seen before:

https://hudson.mozilla.org/job/socorro/243/testReport/junit/socorro.unittest.cron.testSignatureProductdims/TestSignatureProductdims/testPopulateSignatureProductdims/

Error Message

day is out of range for month
-------------------- >> begin captured logging << --------------------
builds: INFO: There were no priority_job tables to close
--------------------- >> end captured logging << ---------------------

Stacktrace

Traceback (most recent call last):
  File "/usr/lib/python2.6/unittest.py", line 279, in run
    testMethod()
  File "/var/lib/hudson/jobs/socorro/workspace/trunk/socorro/unittest/cron/testSignatureProductdims.py", line 163, in testPopulateSignatureProductdims
    makeBogusSignatureProductdims(me.conn, me.cur)
  File "/var/lib/hudson/jobs/socorro/workspace/trunk/socorro/unittest/cron/testSignatureProductdims.py", line 42, in makeBogusSignatureProductdims
    window_end = datetime.datetime(today.year, today.month, today.day - 1, 2, 0, 0).isoformat(' ')
ValueError: day is out of range for month
-------------------- >> begin captured logging << --------------------
builds: INFO: There were no priority_job tables to close
--------------------- >> end captured logging << ---------------------

Comment 17

[(not currently active) Ted Mielczarek]

Busted date math. :) You probably want to change that to something like:
window_end = datetime.datetime.fromordinal((today - datetime.timedelta(days=1)).toordinal())

Comment 18

[Robert Helmer [:rhelmer]]

Tracking failures in this bug is a little messy; let's use it for tracking.

Once we're consistently green, let's close this out and just file individual bugs for different failures as appropriate.

Comment 19

[Robert Helmer [:rhelmer]]

(In reply to comment #18)
> Tracking failures in this bug is a little messy; let's use it for tracking.

Er.. that didn't come out right :) ITIM: Let's use this as a "tracking bug" instead of trying to solve problems in comments.

Comment 20

[Robert Helmer [:rhelmer]]

We've been green for a while, and stage now only pulls builds that pass all unittests which gives us an incentive to stay that way ;)