1020458 - Jetpack should handle timeouts/hangs correctly - outputting a TBPL parsable failure message with test name

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Ed Morley [:emorley]]

Due to a lack of handling of timeouts/hangs in the jetpack harness.

from:
{
TEST-START | layout-change/main.test compatibility
TEST-PASS | layout-change/main.test compatibility | sdk/self -> self
TEST-PASS | layout-change/main.test compatibility | sdk/tabs -> tabs

command timed out: 7200 seconds elapsed, attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=7200.016568
TinderboxPrint:testaddons<br/>101/0
TinderboxPrint: testaddons<br/><em class="testfail">timeout</em>
========= Finished 'python jetpack/bin/cfx ...' warnings (results: 1, elapsed: 2 hrs, 3 secs) (at 2014-06-04 08:49:38.340247) =========
}

we get this as a TBPL failure summary:
"command timed out: 7200 seconds elapsed, attempting to kill"

Whereas we would expect:
{
...
TEST-PASS | layout-change/main.test compatibility | sdk/tabs -> tabs
TEST-UNEXPECTED-FAIL | layout-change/main.test compatibility | Test timed out
}

or for bonus points:
{
...
TEST-UNEXPECTED-FAIL | layout-change/main.test compatibility | Test timed out (after sdk/tabs -> tabs)
}

For more examples, see bug 942111 and bug 926264.

Without this fix, jetpack doesn't currently meet:
https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#6.29_Outputs_failures_in_a_TBPL-starrable_format

Comment 1

[Tomislav Jovanovic :zombie]

i just filed bug 1020401, which is related.

the second problem is that this is (most likely) a firefox hang. that means the JS part of our test harness doesn't get to end the test with a nice parseable message.

the possible solution (after we fix 1020401) is to parse for the last TEST-START from the Python part of our test harness, but this is not ideal as:

1) we are switching away from python code, and

2) this looks like effort duplication, as i presume other test harnesses already do/would have to do something similar..

Comment 2

[Tomislav Jovanovic :zombie]

sorry, didn't mean to do that..

Comment 3

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Ed Morley [:edmorley UTC+0] from comment #0)
> Due to a lack of handling of timeouts/hangs in the jetpack harness.
> 
> from:
> {
> TEST-START | layout-change/main.test compatibility
> TEST-PASS | layout-change/main.test compatibility | sdk/self -> self
> TEST-PASS | layout-change/main.test compatibility | sdk/tabs -> tabs
> 
> command timed out: 7200 seconds elapsed, attempting to kill
> process killed by signal 9
> program finished with exit code -1
> elapsedTime=7200.016568
> TinderboxPrint:testaddons<br/>101/0
> TinderboxPrint: testaddons<br/><em class="testfail">timeout</em>
> ========= Finished 'python jetpack/bin/cfx ...' warnings (results: 1,
> elapsed: 2 hrs, 3 secs) (at 2014-06-04 08:49:38.340247) =========
> }
> 
> we get this as a TBPL failure summary:
> "command timed out: 7200 seconds elapsed, attempting to kill"
> 
> Whereas we would expect:
> {
> ...
> TEST-PASS | layout-change/main.test compatibility | sdk/tabs -> tabs
> TEST-UNEXPECTED-FAIL | layout-change/main.test compatibility | Test timed out
> }
> 
> or for bonus points:
> {
> ...
> TEST-UNEXPECTED-FAIL | layout-change/main.test compatibility | Test timed
> out (after sdk/tabs -> tabs)
> }
> 
> For more examples, see bug 942111 and bug 926264.
> 
> Without this fix, jetpack doesn't currently meet:
> https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#6.
> 29_Outputs_failures_in_a_TBPL-starrable_format

So we are printing `TEST-UNEXPECTED-FAIL` for unexpected timeouts when running the testpkgs suite, but we were not for testaddons and the latter should be done.

This happened because the --parseable was implemented before we implemented testaddons, and as a result of the way testaddons were implemented this issue was overlooked.

Comment 4

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Ed Morley [:edmorley UTC+0] from comment #0)

> or for bonus points:
> {
> ...
> TEST-UNEXPECTED-FAIL | layout-change/main.test compatibility | Test timed
> out (after sdk/tabs -> tabs)
> }

I think we should make this a separate bug.

Comment 5

[Erik Vold [:erikvold] (please needinfo? me)]

Attachment: Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1506

Comment 7

[Tomislav Jovanovic :zombie]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #4)
> > or for bonus points:
> > TEST-UNEXPECTED-FAIL | layout-change/main.test compatibility | Test timed out (after sdk/tabs -> tabs)
> 
> I think we should make this a separate bug.

we already have bug 1003253, but i was planning to implement that in-firefox, with JS part of our test harness, which wouldn't cover cases like this (firefox hang, killed by python).

handling these cases would require python parsing firefox output looking for TEXT-XXX messages, which might not we worth doing in soon-to-be-retired code?  plus, it was only for bonus points, so.. 

Erik, you agree?

Comment 8

[Ed Morley [:emorley]]

Thank you for fixing this :-)

Comment 9

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/f8885af86db3986254d645f8ef59d5f277a7f298
Bug 1020458 - Jetpack should handle timeouts/hangs correctly - outputting a TBPL parsable failure message with test name

https://github.com/mozilla/addon-sdk/commit/59eda68fa4282e96269a314b4b8c6b090ae27909
Merge pull request #1506 from erikvold/1020458

Bug 1020458 - Jetpack should handle timeouts/hangs correctly r=@jsantell

Comment 11

[Dave Townsend [:mossop]]

Is this fixed now?

Comment 12

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Dave Townsend [:mossop] from comment #11)
> Is this fixed now?

I was only waiting for an uplift to aurora in bug 1026636 before closing this one.