Add some module tests

RESOLVED FIXED in Firefox 45

Status

()

Core
JavaScript Engine
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

(Blocks: 1 bug)

unspecified
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(4 attachments)

(Assignee)

Description

2 years ago
This bug is to commit a bunch of module test cases I have sitting in my patch queue.
(Assignee)

Comment 1

2 years ago
Created attachment 8684231 [details] [diff] [review]
test-ambiguous-exports

Ambiguous exports occur when we use |export *| to export the same name from more than one module.  Import or export of such a name is an error, but a module namespace ignores ambiguous exports.
Attachment #8684231 - Flags: review?(shu)
(Assignee)

Comment 2

2 years ago
Created attachment 8684232 [details] [diff] [review]
test-assign-import

Test that assigning to or deleting the various types of imports is an error.
Attachment #8684232 - Flags: review?(shu)
(Assignee)

Comment 3

2 years ago
Created attachment 8684234 [details] [diff] [review]
test-many-import-exports

Check that nothing goes awry when we have > 1000 imports or exports.
Attachment #8684234 - Flags: review?(shu)
(Assignee)

Comment 4

2 years ago
Created attachment 8684294 [details] [diff] [review]
test-cyclic-imports

> Just checking: imported bindings can never be uninitialized (thus triggering TDZ) in the target module at this point, right?

Unfortunately yes they can, and this is the case - cyclic imported variable dependencies.
Attachment #8684294 - Flags: review?(shu)

Comment 5

2 years ago
Comment on attachment 8684231 [details] [diff] [review]
test-ambiguous-exports

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

::: js/src/jit-test/tests/modules/ambiguous-star-export.js
@@ +40,5 @@
> +// Check that namespace objects include only non-ambiguous names.
> +let m = parseModule("import * as ns from 'c'; ns;");
> +m.declarationInstantiation();
> +let ns = m.evaluation();
> +let names = Object.keys(ns).sort();

sort() doesn't seem important here since your tests below use 'in'
Attachment #8684231 - Flags: review?(shu) → review+

Updated

2 years ago
Attachment #8684232 - Flags: review?(shu) → review+

Comment 6

2 years ago
(In reply to Jon Coppeard (:jonco) from comment #4)
> Created attachment 8684294 [details] [diff] [review]
> test-cyclic-imports
> 
> > Just checking: imported bindings can never be uninitialized (thus triggering TDZ) in the target module at this point, right?
> 

What patch review did I say that in? I made that comment because there wasn't code that correctly dealt with imported bindings being uninitialized.

Comment 7

2 years ago
Comment on attachment 8684234 [details] [diff] [review]
test-many-import-exports

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

General note for future tests, using template strings over string concatenation would make the building up of the module source texts more readable.
Attachment #8684234 - Flags: review?(shu) → review+

Updated

2 years ago
Attachment #8684294 - Flags: review?(shu) → review+

Comment 8

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ff488cc4d44
https://hg.mozilla.org/integration/mozilla-inbound/rev/be11a4215c0b
https://hg.mozilla.org/integration/mozilla-inbound/rev/d92cfc42f114
https://hg.mozilla.org/integration/mozilla-inbound/rev/012f6c8917e5
(Assignee)

Comment 9

2 years ago
(In reply to Shu-yu Guo [:shu] from comment #6)
> What patch review did I say that in? I made that comment because there
> wasn't code that correctly dealt with imported bindings being uninitialized.

Right, this was in bug 1219288.  But it's a situation that wasn't previously exercised by our tests so I thought it was worth adding a test case right away.

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8ff488cc4d44
https://hg.mozilla.org/mozilla-central/rev/be11a4215c0b
https://hg.mozilla.org/mozilla-central/rev/d92cfc42f114
https://hg.mozilla.org/mozilla-central/rev/012f6c8917e5
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.