566274 - Describe the reason of failed tests on verbose mode

Status: RESOLVED FIXED

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

Description

[:Felipe Gomes (needinfo for replies!)]

Attachment: patch

This is the follow-up I promised on 564478. This patch adds a simple description of each test failure to the list of failed tests on verbose mode. As an example, it will show output like this:

The following tests failed:
  test-felipe.test1: failure, timed out
  test-felipe.test2: exception

The possible values are: failure (for failed assertions), timed out, exception, empty test (no assertions).

(As a bonus, if only one iteration is run it won't unecessarily show "Iteration 1" as it was before)

I also took the opportunity to do some clean-up on the code from the other patch to make it more flexible. Specifically, I changed the failedTests array to a testsSummary array that contains info from all tests (not the only ones that failed), so it can be used in the future to pass arbitrarily more info about each test.


One issue I had with the implementation is that copying the results from each test iteration to the global results produced a leak, so that's why the code on harness.js makes a copy of the information passed to it.

Comment 1

[Atul Varma [:atul]]

Comment on attachment 445649 [details] [diff] [review]
patch

This looks good--the only thing I'd consider changing, perhaps, is renaming logTestFailed() to _logTestFailed() to indicate that it's not meant to be a public method of the TestRunner class.

If it *is* meant to be a public method, OTOH, we should document it in the markdown docs.

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Attachment: patch for check-in

(In reply to comment #1)
> (From update of attachment 445649 [details] [diff] [review])
> This looks good--the only thing I'd consider changing, perhaps, is renaming
> logTestFailed() to _logTestFailed() to indicate that it's not meant to be a
> public method of the TestRunner class.
> 
> If it *is* meant to be a public method, OTOH, we should document it in the
> markdown docs.

Thanks Atul. That function is not meant as a public method, so I've changed it to be _logTestFailed

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Fixed by changeset https://hg.mozilla.org/labs/jetpack-sdk/rev/9955a8ba6836.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".