Closed Bug 1222446 Opened 5 years ago Closed 5 years ago

Add some module tests

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: jonco, Assigned: jonco)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

This bug is to commit a bunch of module test cases I have sitting in my patch queue.
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)
Test that assigning to or deleting the various types of imports is an error.
Attachment #8684232 - Flags: review?(shu)
Check that nothing goes awry when we have > 1000 imports or exports.
Attachment #8684234 - Flags: review?(shu)
> 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 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+
Attachment #8684232 - Flags: review?(shu) → review+
(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 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+
Attachment #8684294 - Flags: review?(shu) → review+
(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.
You need to log in before you can comment on or make changes to this bug.