792546 - Move xpcshell test helper code to testing only JS modules

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Gregory Szorc [:gps]]

Attachment: Part 1: Move rotary engine to standalone module, v1

Sync has a lot of code in head.js and other support files. As I am refactoring things, I'm finding that head.js and related are doing things for every test that I don't necessarily want. It also makes it hard to track down failures in head.js because the xpcshell test runner doesn't give nice errors when there's a failure before tests are actually executed.

Anyway, I've wanted to refactor head.js into testing-only JS modules for a while. I'm just going to throw a few patches together to do it.

First patch is pretty trivial. Please note that I'm installing tests in services/sync rather than services-sync. We decided that we want to use services/sync going forward. This is as good of a place to start as any.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Part 2: Move fake services to module, v1

Comment 2

[Richard Newman [:rnewman]]

Comment on attachment 662763 [details] [diff] [review]
Part 2: Move fake services to module, v1

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

Also, ugh, that's some shoddy code. I'm sure I wrote some of it, too.

Comment 3

[Gregory Szorc [:gps]]

Attachment: Part 3: Move logging functions into standalone module, v1

This is actually part of services/common, but meh.

Comment 4

[Gregory Szorc [:gps]]

Attachment: Part 4: Move some misc functions into a utils module, v1

I moved the low-hanging fruit.

Comment 5

[Gregory Szorc [:gps]]

Attachment: Part 5: Prune module imports, v1

I think this about does it for low-hanging fruit. Cleaning up other parts of head.js and the other always-included .js files will take a bit of refactoring, including to the xpcshell runner itself and to the extensions tests.

Comment 6

[Richard Newman [:rnewman]]

Comment on attachment 663524 [details] [diff] [review]
Part 4: Move some misc functions into a utils module, v1

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

::: services/sync/modules-testing/utils.js
@@ +10,5 @@
> +  "setBasicCredentials",
> +  "SyncTestingInfrastructure",
> +  "TEST_CLUSTER_URL",
> +  "TEST_SERVER_URL",
> +  "waitForZeroTimer",

Can we put these in a sane order? That is,

  "CONSTANTS",
  "Classes",
  "methods"

?

@@ +26,5 @@
> +
> +function waitForZeroTimer(callback) {
> +  // First wait >100ms (nsITimers can take up to that much time to fire, so
> +  // we can account for the timer in delayedAutoconnect) and then two event
> +  // loop ticks (to account for the Utils.nextTick() in autoConnect).

Might as well lift this to a JavaDoc.

@@ +39,5 @@
> +  }
> +  CommonUtils.namedTimer(wait, 150, {}, "timer");
> +}
> +
> +

Whitespace.

@@ +44,5 @@
> +function setBasicCredentials(username, password, syncKey) {
> +  let ns = {};
> +  Cu.import("resource://services-sync/service.js", ns);
> +
> +  let auth = ns.Service.identity;

This should be on the list of singleton usages to remove. Should be fine for now, because Service still is, but…

Comment 7

[Richard Newman [:rnewman]]

Comment on attachment 663532 [details] [diff] [review]
Part 5: Prune module imports, v1

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

Tests pass. Roll on.

Comment 8

[Gregory Szorc [:gps]]

https://hg.mozilla.org/services/services-central/rev/234c79128437
https://hg.mozilla.org/services/services-central/rev/da562c88998f
https://hg.mozilla.org/services/services-central/rev/6fc02deb63da
https://hg.mozilla.org/services/services-central/rev/b85f209e6072
https://hg.mozilla.org/services/services-central/rev/4a49a6f54e23

Comment 9

[Richard Newman [:rnewman]]

https://hg.mozilla.org/mozilla-central/rev/234c79128437
https://hg.mozilla.org/mozilla-central/rev/da562c88998f
https://hg.mozilla.org/mozilla-central/rev/6fc02deb63da
https://hg.mozilla.org/mozilla-central/rev/b85f209e6072
https://hg.mozilla.org/mozilla-central/rev/4a49a6f54e23