RFC: Testing Module Semantics

[jugglinmike]

@aklein @boingoing and @syg I understand you are all working on ES2015 module implementations, so if you have a moment, your input would be especially helpful here. Thanks!

Because ES2015 intentionally omits necessary details around module resolution (see HostResolveImportedModule), functional tests cannot be written based on the specification alone.

In order to author meaningful tests, Test262 contributors and runtime implementors have to come to an agreement for additional semantics. It should be clear that conformance to this contract is a prerequisite only for Test262 and not for ECMAScript compliance generally.

I recently proposed a new document named INTERPRETNG.md to house exactly this kind of information. If that is accepted, we can extend it with the following text:

Modules. Test262 includes tests for ECMAScript 2015 module code. In order to interpret these tests, consumers must expose an implementation of HostResolveImportedModule that conforms to the following specification:

• The normal return value must be an instance of a Source Text Module
  Record.
• If a Module Record corresponding to the pair referencingModule, specifier
  does not exist or cannot be created, a SyntaxError must be thrown.

But the most contentious detail (as I see it) is resolving module specifiers. The design should be as simple as possible in order to limit the amount of custom behavior necessary to execute the tests. We can take a cue from common file system/web conventions to produce a design that can work out-of-the-box for many consumers.

Here's what I have in mind:

All module specifiers used by Test262 begin with the character sequence ./. The remaining characters should be interpreted as the name of a file within the same directory as the file under test.

For example, consider a test file located at test/language/import/nested/index.js with the following contents:

import * as ns from './dep.js';

Implementors should attempt to resolve this module specifier by loading a file located at test/language/import/nested/dep.js.

I think this will give authors just enough power to test the semantics while promoting a healthy file structure, and it limits the imposition on consumers. One potential issue is that it means Test262 will define non-test files alongside test files. I can think of three ways around this:

1. Additional meta-data for files that do not describe tests with (e.g. flags: [not-a-test])
2. Different resolution semantics: define a top-level "fixtures" directory, and
   specify that loaders should resolve all modules from there.
3. Ignore the "not a test" distinction and just format fixture files as though
   they were valid assertion-less tests

#2 seems too onerous for implementors (it also may preclude meaningful tests since only fixture files--not test files--will be able to import themselves). #3 seems easiest for everyone, but the informality/inefficiency of it irks me. I like #1 the best.

Does this sound good? Is there some detail I'm missing? Or some way what I'm proposing could be improved?

[domenic]

Regarding the mixing of test and non-test files, I'd suggest _fixtures subdirectories and then you can do import * as ns from './_fixtures/dep.js.

[jugglinmike]

From the perspective of the test/ directory, files will still be mixed in that case, but I can see how filtering will be simplified because it can be done by file name and not file contents.

That said, it may also be a little harder to implement across platforms. Is it possible that a Windows-based host would have trouble with the path separator? The fix would be easy, but it would also raise the bar for getting the tests running.

Maybe we could get away with a file name prefix:

import * as ns from './_fixture_dep.js';

Although a suffix might be even better because it would allow for related files to appear together in lexicographically-sorted directory listings.

[jugglinmike]

Although I suppose that by using a ./ prefix, we're already committing to a separator. (I've been trying to think about the specifier as both a traditional file system path and a generic identifier without those semantics, and it's tricky for me to keep that straight.)

[littledan]

@ajklein

[jugglinmike]

Oh, thanks @littledan! GitHub's "aklein" is a doppleganger or IRC's "aklein".

[ajklein]

From an implementation point of view, using "./" as the prefix and requiring it to mean current directory seems fine to me (though I think it would likely also be fine to require a subdirectory there, if that was easier for the test runner). I don't have enough experience with the details of the test runner to be able to choose meaningfully between 1-3, although my inclination, too, is for something like (1).

[jugglinmike]

I'm starting to think that we don't need to specify the behavior when
attempting to load a non-existent file. There is only one line in the
specification where this is observable (for Source Text Module Records), and a
SyntaxError from a non-existent file would be indistinguishable from a
SyntaxError from an existent file containing invalid code. So for testing
purposes (and in order to impose as little as possible on implementers), I
think we can elide this detail.

This calls for a tightening up of the rest of the language, though. We'll want
to remove this line:

The normal return value must be an instance of a Source Text Module Record.

And extend the description of file loading:

All module specifiers used by Test262 begin with the character sequence ./.
The remaining characters should be interpreted as the name of a file within
the same directory as the file under test. The contents of this file must be
interpreted as UTF-8-encoded text and supplied to the Source Text Module
Record's ParseModule abstract operation. The result of that operation must be
returned by the implementation-defined HostResolveImportedModule directly.

[jugglinmike]

For anyone following along: I've submitted a few pull requests to test this part of the spec, following the approach we've been discussing here:

• gh-560 Module code: syntax validation
• gh-561 Module code: early errors
• gh-562 Module semantics: declaration instantiation
• gh-563 Module semantics: evaluation

I chose to demarcate non-test files with a trailing underscore (_), as in eval-gtbndng-indirect-update-as_.js.

There's a lot of code there, but I'd really appreciate it if anyone here could take a look and share some feedback!

[bterlson]

I don't like the convention of trailing underscore. It seems easy to miss. Leading underscore seems better, but what about a different file extension entirely?

[jugglinmike]

I'd prefer that the demarcation come after the "traditional" file name. This ensures that related files will be displayed together in most (all?) views of the file hierarchy. (For instance, the non-test-file eval-gtbndng-indirect-update-as_.js mentioned above is referenced by a test file named eval-gtbndng-indirect-update-as.js)

I think there's good reason to avoid a new file extension, though: it may confuse syntax highlighters on GitHub or in contributors' editors/IDEs.

Would you be satisfied with a "louder" suffix? Maybe *_FIXTURE.js? Or *_NT.js for "Not Test"?

[bterlson]

Good points (though I don't think the highlighting bit is too important, it's easy to change file mode in any decent editor). _FIXTURE or something would be better in my opinion, especially now that MAX_PATH is fixed :-P

[jugglinmike]

Alrighty--I've updated the relevant pull requests to use _FIXTURE as a naming prefix for non-test files.

[leobalter]

from #552 to #676, I believe this is already defined. Let's reopen if necessary.