1129336 - Remove nonstandard let blocks from testing/xpcshell

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: testing-xpcshell.patch

let blocks are a nonstandard SpiderMonkey feature we would like to remove (in bug 1023609). This patch removes the let blocks in the testing/ subtree.

The attached patch excludes whitespace changes because most of the patch is unindenting blocks of unchanged code from the removed let blocks. This made the important bits of the patch harder to review.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8558998 [details] [diff] [review]
testing-xpcshell.patch

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

::: testing/xpcshell/head.js
@@ +52,5 @@
>  // not connected to a network.
> +let ios2 = Components.classes["@mozilla.org/network/io-service;1"]
> +                    .getService(Components.interfaces.nsIIOService2);
> +ios2.manageOfflineStatus = false;
> +ios2.offline = false;

Top level declarations in xpcshell/head.js are exposed to every test file, so if nonstandard let blocks work the way I expect, this introduces a new variable we'd probably rather keep hidden.

Comment 2

[Chris Peterson [:cpeterson]]

(In reply to Chris Manchester [:chmanchester] from comment #1)
> Top level declarations in xpcshell/head.js are exposed to every test file,
> so if nonstandard let blocks work the way I expect, this introduces a new
> variable we'd probably rather keep hidden.

OK. I can change this code to hide the ios variable in a no-op {} block or an IIFE.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

(In reply to Chris Peterson [:cpeterson] from comment #2)
> (In reply to Chris Manchester [:chmanchester] from comment #1)
> > Top level declarations in xpcshell/head.js are exposed to every test file,
> > so if nonstandard let blocks work the way I expect, this introduces a new
> > variable we'd probably rather keep hidden.
> 
> OK. I can change this code to hide the ios variable in a no-op {} block or
> an IIFE.

A no-op {} block would be ok with me!

Comment 4

[Chris Peterson [:cpeterson]]

Attachment: testing-xpcshell-v2.patch

Patch v2: unfold the `let (ios = ...)` let block into a no-op {} block instead of renaming `ios` to `ios2`.

Comment 5

[Chris Peterson [:cpeterson]]

Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/a1cd935b313d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a1cd935b313d