test262-harness
test262-harness copied to clipboard
All tests in repo fail if any test in repo has an extraneous `/*---` sequence
how to replicate:
- Break a test by adding an extra
/*---
. In this case, I've broken type-datetimefield-valid.js in intl402/DisplayNames/prototype/of.
// Copyright 2021 the V8 project authors. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
/*---
esid: sec-Intl.DisplayNames.prototype.of
description: Returns string value for valid `dateTimeField` codes
features: [Intl.DisplayNames-v2]
---*/
var displayNames = new Intl.DisplayNames(undefined, {type: 'dateTimeField'});
[...]
- Run any test anywhere in the repo. The following error message happens:
test262-harness --host-type=jsc --host-path=`which jsc` locales-symbol-length.js
/usr/local/lib/node_modules/test262-harness/node_modules/test262-stream/lib/test-file.js:65
throw new Error(`Error loading frontmatter from file ${test262File.file}\n${e.message}`);
^
Error: Error loading frontmatter from file test/intl402/DisplayNames/prototype/of/type-datetimefield-valid.js
end of the stream or a document separator is expected at line 3, column 5:
esid: sec-Intl.DisplayNames.prototype.of
^
at loadAttrs (/usr/local/lib/node_modules/test262-harness/node_modules/test262-stream/lib/test-file.js:65:13)
at extractAttrs (/usr/local/lib/node_modules/test262-harness/node_modules/test262-stream/lib/test-file.js:81:17)
at new Test262File (/usr/local/lib/node_modules/test262-harness/node_modules/test262-stream/lib/test-file.js:143:18)
at builder (/usr/local/lib/node_modules/test262-harness/node_modules/test262-stream/lib/builder.js:16:16)
at /usr/local/lib/node_modules/test262-harness/node_modules/test262-stream/lib/compile.js:16:9
at FSReqCallback.readFileAfterClose [as oncomplete] (node:internal/fs/read_file_context:68:3)
I spent some time looking into this. The bug, or at least surprising behavior, is that test262-harness parses the metadata out of all the files in the current working directory, even if you pass in one relative filename like in Ben's example. Normally, this is not visible to the user, because it suppresses output from tests that don't match the pattern(s) provided on the command line. But if a test has a syntax error in its metadata, like type-datetimefield-valid.js
did in Ben's example, then you get an error message about a file that you didn't pass as an argument to test262-harness
. This is surprising.
Why does this happen? test262-harness
depends on the test262-stream
library, which supplies a TestStream
constructor that takes an array of paths and returns a readable stream, on which it emits one event per test file that it reads. If a file has a syntax error (or doesn't exist, etc.) it emits an error on the stream. In the TestStream
class in test262-harness itself (which wraps the upstream TestStream
), the error
handler passes through any upstream errors it receives. This means that the entire stream fails if a single error is emitted upstream. I figured this out through printf-debugging: the main run.js file composes two streams, the first one of which (the pipe()
call on line 175) invokes the test262-stream
library to get all the needed paths, generating a stream of test objects; the second one of which (the pipe()
call on line 179) actually runs each test. Because there was an error in the first stream, nothing in the second stream ever ran.
It's not obvious to me what the right solution is. Some possibilities:
- Only pass specific filenames to test262-stream (this call), not entire directories (unless the user actually specified the whole directory on the command line). The comment on
patternsToDirectories()
intest-stream.js
explains pretty well why this isn't already being done. However, it would be more robust for test262-harness to expand any globs and pass the resulting filenames to test262-stream rather than passing a superset of the desired files by passing in an enclosing parent directory for anything containing globs. - Alternately, the error handler in
test-stream.js
(here) could be modified to ignore errors that relate to files the user didn't ask to test. This would require modifying thetest262-stream
library as well to throw a more structured exception type with a property specifying the path of the offending file. - A hack to avoid changing the exception type would be to do the same thing, but just have the aforementioned error handler scrape out the filename from
error.message
, which would work at least in the situation Ben ran into. - Or, the error diagnostics could simply be improved to highlight the filenames with different colors, or to print a list of files being read at the beginning... not sure what the right UX is, but anything that helps explain why you're getting an error for
type-datetimefield-valid.js
when you only wanted to runlocales-symbol-length.js
.
I was going to try to fix this, until I realized it wasn't obvious what the right solution was.
In any case, the bug title should probably be something more general, like "syntax error in one test causes all tests in the same directory to fail."