538058 - python callbacks don't work in regular mozmill tests

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Jonathan Griffin (:jgriffin)]

Currently python callbacks only work in mozmill-restart tests.  If you attempt to use them in a regular mozmill test, they're just ignored; they never execute, but they don't cause a test failure either.

Comment 1

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

Right, it's only defined in MozmillRestart:
http://github.com/mikeal/mozmill/blob/master/mozmill/__init__.py#L244

Mikeal, would this be tricky or is a simply move over to the Mozmill class possible?

Comment 2

[Jonathan Griffin (:jgriffin)]

Attachment: patch

Here's a patch which adds python callbacks to plain mozmill tests.  The only difference with the mozmill-restart version is that the callbacks are expected to be located in a file /path/to/test_something_callbacks.py, for a test /path/to/test_something.js.

What's the usual process for getting these reviewed?  Using Bugzilla, or just pushing to my git fork and requesting a pull from Mikeal?

Comment 3

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

Mikeal is only watching for pull requests on Github so the second path would be the right one. But always mention those requests in the appropriate bug.

I have only one comment:

* fire_python_callback has only to be defined in Mozmill. There is no need for overriding this function in MozmillRestart, or?

Comment 4

[Jonathan Griffin (:jgriffin)]

Thanks, I'll do it via github.

> * fire_python_callback has only to be defined in Mozmill. There is no need for
> overriding this function in MozmillRestart, or?

The two copies of fire_python_callback are actually slightly different, hence the override.

Comment 5

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

Just push this to a branch on your GitHub fork and I can take a look at it.

Comment 6

[Jonathan Griffin (:jgriffin)]

Attachment: patch v2

I've shuffled some code around a bit to avoid having the redundant code Henrik mentioned.  I'll push to my git fork.

Comment 7

[Jonathan Griffin (:jgriffin)]

Pushed to the 'callbacks' branch of my github fork: http://github.com/jonallengriffin/mozmill/commit/ab322bf641c1f12b5092fbf35c04dbcb36f4809a

Comment 8

[Jonathan Griffin (:jgriffin)]

Added a unit test, and fixed a bug:
http://github.com/jonallengriffin/mozmill/commit/90be273ab59abbf8724ad0c43a0dd8c6d2914bf3

Comment 9

[Jonathan Griffin (:jgriffin)]

Attachment: screenshot

Whimboo and I discussed adding something to the Mozmill IDE to warn users if they attempt to run a test using python callbacks, since those tests won't work correctly in the IDE.  Attached is a screenshot of one possible mechanism (which only takes a 3-line change to frame.js).  Whimboo, what do think?  Another possibility is to show an error and prevent those tests from running, I'm not sure it makes any sense to allow the test to run at all.

Comment 10

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

I think you should just throw an exception and have a good message in the exception.

The exception information will show up in the output in the IDE. A dialog is kind of silly since most of the time they will be running a group of tests and just need to know to not care about this specific failure.

Comment 11

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

So with the unit test in place I run it with the callbacks branch from Jonathan. So one thing I don't understand is why don't we log any failures which happen in the test results?

Comment 12

[Jonathan Griffin (:jgriffin)]

Attachment: screenshot v2

I've pushed a change to my fork (http://github.com/jonallengriffin/mozmill/commit/9f5ac6b560b4b70ab6aa635f8e62495e8623541d) which throws an exception when someone attempts to run a test using a python callback from the IDE.  The resulting UI is shown in this screenshot.

Comment 13

[Jonathan Griffin (:jgriffin)]

> So one thing I don't understand is why don't we log any failures
> which happen in the test results?

I'm not exactly sure what you mean by this, but Mikeal says elsewhere that it isn't possible for python callbacks to propagate messages back to the mozmill js framework.

Comment 14

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

I know that we cannot propagate the failures back to Mozmill at the moment but the callbacks are executed on the Python end and I wonder if it could be possible to collect failures there and attach them to the final results array.

But this is probably a follow-up bug.

Comment 15

[cmtalbert]

Adding Heather to get her ideas on UI since this has a UI component.

Comment 16

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

It's not just a problem of propogating the failures, that problem is actually fixable. The bigger issue is that the python layer callbacks are added in the command line test framework setup which means that you can't even start the tests from the IDE and expect the Python callbacks to work.

The only way to fix this would be to move the entire callback system to a different Python object and stick the initialization code after you start the runner. This is not a small change and will involve more than just moving the callbacks but moving all the wait for active jsbridge code to another area as well. I would suggest punting this UI work and finish up what is there first.

Comment 17

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

I merged jgriffin's latest code, throwing an exception when run from the IDE, to my master.

Comment 18

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

Finally I was able to test this new feature now. Do we have a chance to give a more detailed error report when --show-errors is used? At the moment we only say in which function the exception has been raised but not which line of code.

Comment 19

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

This has been fixed in Mozmill 1.4. I will file a follow-up bug for the more detailed exception message.