Refactor mozmill to allow exporting driver

RESOLVED FIXED

Status

Testing Graveyard
Mozmill
RESOLVED FIXED
6 years ago
a year ago

People

(Reporter: ahal, Assigned: Jeff Hammel)

Tracking

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
I've done some work on this already. There are three steps to this.

1) Apply some form of the diff found here: https://github.com/ahal/mozmill/commit/eb02a3311c0f3e10abc3af5d6f409cd93b10fa68
I will work on un-bitrotting and making this nicer.

2) Move the driver related files out of the modules folder and into its own 'driver' folder. Some thought will need to go into what to do with stdlib files required by both.

3) [longer term] Write a makefile that compiles the files in the driver folder into a single mozmill.js (like jquery does) which can easily be dropped into any chrome javascript.
(Reporter)

Updated

6 years ago
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
(Reporter)

Comment 1

6 years ago
Created attachment 567571 [details] [diff] [review]
Patch 1.0 - Decouple the driver from the test harness

In this definition, the driver is controller.js, elementslib.js, mozelement.js, mozmill.js and init.js. The test harness is basically frame.js.

This patch re-factors mozmill such that none of the driver files reference any of the test harness files. This allows the driver files to be isolated and used in other applications such as peptest. It is done using the observer design pattern.

This patch only addresses the first step in the above comment. Once this is approved and committed we can maybe move directories around, but I'd rather do it incrementally.
Attachment #567571 - Flags: review?(jhammel)
(Reporter)

Comment 2

6 years ago
Created attachment 567573 [details] [diff] [review]
Patch 1.1 - Decouple driver from test harness

Forgot to add a file to the diff, see comment on previous patch.
Attachment #567571 - Attachment is obsolete: true
Attachment #567571 - Flags: review?(jhammel)
(Reporter)

Updated

6 years ago
Attachment #567573 - Flags: review?(jhammel)
(Assignee)

Comment 3

6 years ago
I'll have to look at the patch closer to see what to do with it.  On first glance, though, it seems like frame.MozmillMessageListener is an unnecessary abstraction.
(Assignee)

Comment 4

6 years ago
Created attachment 567817 [details] [diff] [review]
modified version of patch
(Assignee)

Comment 5

6 years ago
Comment on attachment 567573 [details] [diff] [review]
Patch 1.1 - Decouple driver from test harness

After talking to :ahal, a few things:

- listener.update() should just be listener()
- listeners are keyed by event type; we could have global listeners but do not yet
- MozmillMsgListener is an unnecessary abstraction; just move this code into events
Attachment #567573 - Flags: review?(jhammel) → review-
(Assignee)

Comment 6

6 years ago
Created attachment 567885 [details] [diff] [review]
counter-proposal
Assignee: ahalberstadt → jhammel
Attachment #567817 - Attachment is obsolete: true
Attachment #567885 - Flags: review?(ahalberstadt)
(Reporter)

Comment 7

6 years ago
Comment on attachment 567885 [details] [diff] [review]
counter-proposal

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

Yeah, I like this a lot better than the original.

So once this gets checked in I can throw together a patch that re-factors the directory structure. If I remember correctly, it was mostly just changing import statements.
Attachment #567885 - Flags: review?(ahalberstadt) → review+
(Assignee)

Comment 8

6 years ago
pushed to master: https://github.com/mozautomation/mozmill/commit/1c4d1e6f5599e3c54e71c7c1875973a947e801c2

Follow up issues should be separately filed
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.