Include test "compilation" logic #7
Conversation
The functionality previously delegated to the externally-maintained `test262-compiler` module are core to this module's behavior. Include that source code in order to better facilitate maintenance. The `test262-compiler` module exposes a command-line interface uses the`yargs` module. Because that feature is unnecessary in this context, this change also reduces the number of dependencies from 109 to 62.
The `$LOG` function is not referenced by the Test262 project's "interpreting" instructions and should thereefor not be injected into test sources.
The Test262 project "interpreting" guidelines include the following requirement: > [...] > > - **`async`** - The file `harness/doneprintHandle.js` must be > evaluated in the test realm's global scope prior to test execution. Satisfy this requirement by loading the provided file and injecting it only in those tests which specify `async` metadata flag.
The Test262 project "interpreting" guidelines include the following requirement: > ### Test262-Defined Bindings > > The contents of the following files must be evaluated in the test realm's > global scope prior to test execution: > > 1. `harness/assert.js` > 2. `harness/sta.js` Satisfy this requirement by loading the provided file and injecting it into the test contents.
|
First of all, I'm sorry for dropping the ball on reviewing this. Secondly, I wonder if removing |
|
I agree that we could also reduce this project's dependencies by modifying the I don't think the "stream"/"compiler" separation is useful, and I expect that maintaining it would just involve making more work for ourselves. npm only lists three dependent projects for |
|
@jugglinmike thanks for clarifying these points for me—I'm on board. |
lib/compilerdeps.js
Outdated
| @@ -28,7 +28,3 @@ function $DONE(err) { | |||
| $262.destroy(); | |||
| } | |||
|
|
|||
| function $LOG(str) { | |||
| print(str); | |||
| } | |||
There was a problem hiding this comment.
I've confirmed that this is not used an any existing test/* or harness/* files in test262
|
Is there any special handling that you want for merging this? |
|
I'd like to see the commits I've submitted landed without squashing since they both import code authored by someone else and then introduce my own modifications. It's your call whether to use a merge commit or not. (Sorry for the delay, I missed the e-mail notifications for your latest comments.) |
|
Thanks @jugglinmike |
Hey @rwaldron,
Since we started, we've been expected to incorporate the source code from the
test262-compilerproject directly. The feature request in gh-2 kind of forces this issue because it describes disabling some (but not all) of the functionality that module provides.One additional benefit (as noted in the first commit on this branch) is that this reduces the total number of Node.js dependencies by close to 50%.
But more importantly: while implementing this, I found that
test262-compilerviolates the Test262 "interpreting" guidelines in a number of ways. In the interest of transparency (so you can see exactly what I modified after importing the code), I've addressed those violations in dedicated commits. I wanted to call this out because it's possible that those inconsistencies are actually required bytest262-harness. If that's the case, then removing them might make it difficult to usetest262-streamfromtest262-harness. I don't believe this is the case, but I'd appreciate it if you could verify.Thanks!