512535 - Manual throw not providing sufficient information about failures

Status: VERIFIED WONTFIX

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

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

While checking the reports we will send to the results server in the future I have noticed that using throw in our tests we fail to set the state for the test. That's why we should use controller.assertJS.

By going this way we sadly will not have the chance to specify a comment for the failing case. Mikeal, can we add another parameter to this function? Only that way we can create a helpful error comment.

Comment 1

[Mikeal Rogers [:mikeal] (mrogers@)]

What do you mean by "set state for the test". The failure should be there, and the exception information should be there, if not this is a bug.

We can certainly improve this experience and get more information, but I don't think the answer is assertJs.

Once I get a better idea of what you would like to see we can discuss how to implement it.

Comment 2

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

Using assert functions from controller we have the elements info in the output of the failed frame. That's not the case for assertJS. Having a lot of those checks inside a test it will be hard to identify the line which failed by just looking at the output / report. Since we don't have an element we operate on we should pass more information to this assert function.

Comment 3

[Mikeal Rogers [:mikeal] (mrogers@)]

assertJs doesn't have the element either, or at least it doesn't have it in any predictable way that we could use to show more output.

I think a better solution here is to start throwing customized exception objects.

Something like this 

throw new controller.Exception({"elem":elem})

And 

throw new controller.Exception({node:document.getElementById("id"), message:"node property does not match"})

We can define a bunch of different keys in the object passed to exception that we know are useful for debugging and make sure they propagate. Also, since it's attached to the controller it has a reference to the window in question. And this makes the code to handle these exceptions pretty easy since all we do is check if it's an instance of Exception.

Thoughts?

Comment 4

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

As discussed on IRC we wanna go this way. Another option which should be possible is just to call the constructor with a simple message like throw new controller.Exception("Failed because..."). That way we can correctly insert a failed frame which will be reported in the output pane and inside the report for brasstacks.

Comment 5

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

With my patch on bug 515072 we pushed another change yesterday which gives assertJS more output. Lets give an example for the testEnableCookies.js module:

Right now we have this calling convention:
> cmController.assertJS(cm.cookieExists({host: '.mozilla.org', name: '__utmz', path: '/'}) == true);

Which will give the following output:
pass :: Controller.assertJS("true")
fail :: Controller.assertJS("false")

No-one will know which test passed/failed here. So changing the the above line will give us more insight:

> cmController.assertJS("subject.cookieExists({host: '.mozilla.rg', name: '__utmz', path: '/'}) == true", cm);

pass :: Controller.assertJS("subject.cookieExists({host: '.mozilla.org', name: '__utmz', path: '/'}) == true")
fail :: Controller.assertJS("subject.cookieExists({host: '.mozilla.org', name: '__utmz', path: '/'}) == true")

You will probably remember it from waitForEval which uses exactly the same method. This will match most of our assertJS usage and which should be updated once the new version has been released. I will take care of it in bug 525189.

We still have to figure out a nice way when comparing simple elements like numbers or strings.

Comment 6

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

Move of Mozmill Test related project bugs to newly created components. You can
filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.

Comment 7

[u279076]

What's the status of this bug given current state of assertions in Mozmill?
Does this fall under the Refactor project?

Comment 8

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

All for the new assertions module! Lets close this bug out.