885141 - Failure in setup* or teardown* methods are not always counted

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

There are some cases when a failure in any setup* or teardown* method is not counted. Here two examples:

1. The test module only has a setupModule or setupTest method which fails
2. The test module has a teardownModule or teardownTest method which fails

It's important to know if there are failures in those methods. This seems to be a regression from bug 639691.

We might want to account those failures to the right test.

Comment 1

[cmtalbert]

If we don't want to count them as specific tests, then I don't see why we'd suddenly care about line by line failures. If the method goes wrong in a big way (throws an exception or something) then I think we will still stop the test at that point. I don't want us to count specific checks inside the setup/teardown functions when they fail but not when they pass. That sounds like it would make things far more confusing.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Nope, that does not work. If there is no test function in a module, failures in setupModule and setupTest will be hidden. Same applies to any teardown function. I thought that with bug 639691 we wanted to account possible failures to the current test.

I will setup some tests for Mutt so that we can figure out what to do.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So the problem here is that line:
https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/extension/resource/modules/frame.js#L225

  if (test.__passes__ &&
      (modulelist.indexOf(test.__name__) != -1)) {

A check for test.__passes__ is not doing what you might expect. It's an empty array in case of no passes but a failure. So we will not determine the correct state here.

The patch is simple. I will have it ready later and some additional tests.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v1

It's kind of a simple fix. I have also added a couple of tests to ensure we don't regress that again.

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 779796 [details] [diff] [review]
Patch v1

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

Looks good, thanks Henrik!

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Landed as:
https://github.com/mozilla/mozmill/commit/31daf5f982c17c9a58bb758a501fffe4f2b535fd

As the investigation has shown this was absolutely a blocker for 2.0. Setting whiteboard entry accordingly.