Closed Bug 1582056 Opened 5 months ago Closed 5 months ago

Convert helper modules to JSM

Categories

(Thunderbird :: Testing Infrastructure, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 71.0

People

(Reporter: darktrojan, Assigned: darktrojan)

References

Details

Attachments

(3 files)

Some of the shared-modules/test-*-helpers.js files can be easily converted to javascript modules since they don't depend on anything.

I haven't made any attempt to improve the actual code, just shifted things around so that they work. I'm trying to keep imports in a pattern:

  • those from chrome://mozmill/content
  • those from resource://testing-common/mozmill
  • other imports

One of the next steps will probably be to move as much as possible from the remaining shared modules into JSMs.

Attachment #9093534 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9093534 [details] [diff] [review]
1582056-mozmill-mochitest-1.diff

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

Can't say it's pretty, but no worse than it was. r=mkmelin

::: mail/test/mozmill/cloudfile/test-cloudfile-attachment-item.js
@@ +26,5 @@
> +  select_attachments,
> +} = ChromeUtils.import(
> +  "resource://testing-common/mozmill/AttachmentHelpers.jsm"
> +);
> +

well.. this is not nice. getting globals from the jsm.

::: mail/test/mozmill/shared-modules/test-subscribe-window-helpers.js
@@ +9,5 @@
> +var MODULE_REQUIRES = ["window-helpers", "folder-display-helpers"];
> +
> +var { input_value, delete_all_existing } = ChromeUtils.import(
> +  "resource://testing-common/mozmill/KeyboardHelpers.jsm"
> +);

In general, would be nicer to just import the whole KeyboardHelpers and call functions on that, like KeyboardHelpers.input_value(....)
Attachment #9093534 - Flags: review?(mkmelin+mozilla) → review+

Still waiting on bug 1582055, so here's the next bit.

Attachment #9093776 - Flags: review?(mkmelin+mozilla)

(In reply to Magnus Melin [:mkmelin] from comment #2)

In general, would be nicer to just import the whole KeyboardHelpers and call
functions on that, like KeyboardHelpers.input_value(....)

I agree, but I'm keeping this to the minimum viable change at the moment (which also makes a few things easier) and I'll come back and change all of the function calls in one giant patch at the end.

Comment on attachment 9093776 [details] [diff] [review]
1582056-mozmill-mochitest-part2-1.diff

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

Looks good! r=mkmelin
Attachment #9093776 - Flags: review?(mkmelin+mozilla) → review+

Hi, what is this good for?
Will only some of the files become .jsms? Won't this just be a bigger mess?

If this was all I was doing then yes, I'd agree with you. But it isn't. I'm now working on a patch to convert all the remaining helper files to JSMs, and when that is done I'll be dismantling Mozmill altogether.

Warning: reading this patch may make you drowsy. Do not drive or operate machinery after reading this patch.

Attachment #9095046 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9095046 [details] [diff] [review]
1582056-mozmill-mochitest-part3-1.diff

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

That's a big patch! I think my page-down key is exhausted.
Attachment #9095046 - Flags: review?(mkmelin+mozilla) → review+

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/0ee0e21ccb1b
part 1 - Convert some Mozmill helper modules to JSMs; r=mkmelin
https://hg.mozilla.org/comm-central/rev/19ae7a021b57
part 2 - Convert test-window-helpers.js to a JSM; r=mkmelin
https://hg.mozilla.org/comm-central/rev/284f2743822f
part 3 - Convert all remaining Mozmill helper modules to JSMs; r=mkmelin DONTBUILD

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 71.0
You need to log in before you can comment on or make changes to this bug.