Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update test262 +test262.properties file (re)generator #930

Merged
merged 17 commits into from
Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ test {
systemProperty 'TEST_OPTLEVEL', System.getProperty('optLevel')
}
systemProperty 'test262properties', System.getProperty('test262properties')
systemProperty 'updateTest262properties', System.getProperty('updateTest262properties')
maxHeapSize = "1g"
testLogging.showStandardStreams = true
// Many tests do not clean up contexts properly. This makes the tests much
Expand Down
2 changes: 1 addition & 1 deletion test262
Submodule test262 updated 4452 files
71 changes: 67 additions & 4 deletions testsrc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ git submodule update

After doing so, the `./gradlew test` command will also execute all tests that are part of the official ECMAScript Test Suite

As Rhino isn't 100% compliant with the latest ECMAScript standard, there is a mechanism to define which tests to run/skip,
through the [test262.properties](test262.properties) file, the format of which is discussed in the [test262.properties format](#test262.properties-format) section

## Optimization levels
By default all tests are run 3 times, at optimization levels -1, 0 and 9.

Expand All @@ -32,14 +35,74 @@ This behavior can be changed through different means:

## Running a specific TestSuite
```
./gradlew test --tests org.mozilla.javascript.tests.Test262SuiteTest
./gradlew test --tests org.mozilla.javascript.tests.Test262SuiteTest
```

## test262.properties format
The [test262.properties](test262.properties) file is used to specify which tests from the official ECMAScript Test Suite to include/exclude,
so that the test suite can pass even though Rhino is not yet 100% standards compliant

The [test262.properties](test262.properties) file:
- lists the subfolders of the [test262](../test262) folder to include or exclude when running tests
- lists the .js test files that are expected to fail per (sub)folder

**Basics**
```
built-ins/Array <- include all the tests in the test262/built-ins/Array folder
from/calling-from-valid-1-noStrict.js <- but expect that this tests fails
```
If `built-ins/Array/from/calling-from-valid-1-noStrict.js` indeed fails, it wont fail the test suite. If it passes, this will be logged while running the test suite
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused about the default behavior. I understand that if "built-ins/Array" is included, then we run all the files in that directory except for the ones called out below. But if "built-ins/Array" is not included in the file, then do we run those tests anyway? The PR comments seemed to indicate that we do.

Copy link
Collaborator Author

@p-bakker p-bakker Jun 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the PR comments are then misleading: both before my changes and now with my changes, only testfiles from the test262/test/ subfolders listed in the test262.property at the top level (i.e. built-ins/Array or language/expressions/function) will be included.

At the bottom of my PR description I did mention that in a future task we should include ALL test262/test subfolders into the test262.properties file when regenerating it, so we dont miss out on newly added folders in the test262 project

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that "nothing" is OK. If there's no .js suffix, then we can assume that it's a prefix, and if there is a .js suffix, then we can assume that it's a fail. I don't feel strongly, but if you ask me, that makes the most sense and is the simplest.


**Skipping entire folders**
```
~built-ins/decodeURI
name.js
S15.1.3.1_A2.4_T1.js
S15.1.3.1_A5.2.js
```
Toplevel folders can be prefixed with a `~` to mark the folder to be skipped entirely, for example because it contains all tests for a feature not yet supported by Rhino or because running the tests take a long time.
Any files listed for a skipped folder will be skipped as well.

**Expecting all files in a (sub)folder to fail**
```
built-ins/Array <-- topLevel folder
prototype/flatMap <-- subfolder under topLevel folder
```
If all files in a subfolder below a topLevel folderare expected to fail, instead of listing all files explicitly, just the path of the folder needs to be included under the topLevel folder

**Comments**
The test262.properties file uses the Java Properties format, with the folder/.js file paths being the property key. The value of each 'property' can be used to store a comment
```
~built-ins/Array All tests on the built-in Array class
prototype/flatMap haven't gotten around to implementing flatMap yet
```

A Java Properties file can also have entire lines as comments, by prefixing the line with either `!` or `#`.
While the test262.properties file does support this (because it is a Java Properties file), such line comments will be lost when (re)generating the test262.properties file!

## Updating the test262.properties file
While the [test262.properties](test262.properties) file could be manually updated, the tooling also comes with a mechanism to (re)generate the file based on the current revision of the test262 submodule and the results of running Test262SuiteTest against this revision on all standard optLevels (-1, 0 & 9)

```
./gradlew test --tests org.mozilla.javascript.tests.Test262SuiteTest --rerun-tasks -DupdateTest262properties [-Dtest262properties=testsrc/myOwn.properties]
```
The .properties file generation can be parameterized to affect the output:
- rollup: include only a single line for a subfolder that contains only failing tests
- stats: include stats on the # of failed and total tests
- unsupported: include files containing tests for unsupported features

These defaults can be overridden by specifying a value for the `generateTest262properties` parameter:
- all: rollup, stats and unsupported all true (default)
- none: rollup, stats and unsupported all false
- [rollup][stats][unsupported]: the ones included will be true

Note: the tests must actually run for the .properties file to be updated. If gradle determines that nothing has changed since the last time the `test` command was run, it won't run the tests. The `--rerun-tasks` argument forces gradle to run all tests

## Running specific tests from the official ECMAScript Test Suite (test262)
As Rhino isn't 100% compliant with the latest ECMAScript standard, there is a mechanism to define which tests to run/skip.
The default is [test262.properties](test262.properties).
The default setup for running the test262 test suite is defined in [test262.properties](test262.properties).

Another .properties file to use can be specified using the `test262properties` commandline property
```
./gradlew test --tests org.mozilla.javascript.tests.Test262SuiteTest -Dtest262properties=testsrc/myOwn.properties
```
```
This allows the creation of a custom .properties file containing for example just the tests for one specific feature being implemented, which allows for (quickly) running just the tests for that specific feature
48 changes: 21 additions & 27 deletions testsrc/org/mozilla/javascript/drivers/TestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import java.util.Arrays;
import java.util.List;
import java.util.Properties;

import org.mozilla.javascript.ContextFactory;

public class TestUtils {
Expand All @@ -31,65 +30,60 @@ public static void setGlobalContextFactory(ContextFactory factory) {
}

public static File[] recursiveListFiles(File dir, FileFilter filter) {
if (!dir.isDirectory())
throw new IllegalArgumentException(dir + " is not a directory");
if (!dir.isDirectory()) throw new IllegalArgumentException(dir + " is not a directory");
List<File> fileList = new ArrayList<File>();
recursiveListFilesHelper(dir, filter, fileList);
return fileList.toArray(new File[fileList.size()]);
}

public static void recursiveListFilesHelper(File dir, FileFilter filter,
List<File> fileList)
{
for (File f: dir.listFiles()) {
public static void recursiveListFilesHelper(File dir, FileFilter filter, List<File> fileList) {
for (File f : dir.listFiles()) {
if (f.isDirectory()) {
recursiveListFilesHelper(f, filter, fileList);
} else {
if (filter.accept(f))
fileList.add(f);
if (filter.accept(f)) fileList.add(f);
}
}
}

public static void addTestsFromFile(String filename, List<String> list)
throws IOException {
public static void addTestsFromFile(String filename, List<String> list) throws IOException {
addTestsFromStream(new FileInputStream(new File(filename)), list);
}

public static void addTestsFromStream(InputStream in, List<String> list)
throws IOException {
public static void addTestsFromStream(InputStream in, List<String> list) throws IOException {
Properties props = new Properties();
props.load(in);
for (Object obj: props.keySet()) {
for (Object obj : props.keySet()) {
list.add(obj.toString());
}
}

public static String[] loadTestsFromResource(String resource, String[] inherited)
throws IOException {
List<String> list = inherited == null ?
new ArrayList<String>() :
new ArrayList<String>(Arrays.asList(inherited));
List<String> list =
inherited == null
? new ArrayList<String>()
: new ArrayList<String>(Arrays.asList(inherited));
InputStream in = JsTestsBase.class.getResourceAsStream(resource);
if (in != null)
addTestsFromStream(in, list);
if (in != null) addTestsFromStream(in, list);
return list.toArray(new String[0]);
}

public static boolean matches(String[] patterns, String path) {
for (int i=0; i<patterns.length; i++) {
for (int i = 0; i < patterns.length; i++) {
if (path.startsWith(patterns[i])) {
return true;
}
}
return false;
}

public static final FileFilter JS_FILE_FILTER = new FileFilter() {
@Override
public boolean accept(File pathname) {
return pathname.getAbsolutePath().endsWith(".js");
}
};

public static final FileFilter JS_FILE_FILTER =
new FileFilter() {
@Override
public boolean accept(File pathname) {
String path = pathname.getAbsolutePath();
return path.endsWith(".js") && !path.endsWith("_FIXTURE.js");
}
};
}
Loading