Improved CI workflows

This improves several things in the CI workflows:

 - More conformant Test262 result generation
 - Benchmarks should now show comments for all users
 - Added Test262 result comparison comments to Pull Requests

.github/workflows/pull_request.yml

@@ -1,4 +1,4 @@
-on: [pull_request]
+on: [pull_request_target]
 name: Benchmarks
 jobs:
   runBenchmark:

.github/workflows/test262.yml

@@ -5,7 +5,7 @@ on:
       - master
     tags:
       - v*
-  pull_request:
+  pull_request_target:
     branches:
       - master
 
@@ -36,7 +36,6 @@ jobs:
 
       # Run the test suite and upload the results
       - name: Checkout GitHub pages
-        if: github.event_name == 'push'
         uses: actions/checkout@v2
         with:
           ref: gh-pages
@@ -45,16 +44,55 @@ jobs:
       - name: Run the test262 test suite
         run: |
           cd boa
-          cargo run --release --bin boa_tester -- -o ../gh-pages/test262
+          mkdir ../results
+          cargo run --release --bin boa_tester -- run -o ../results/test262
           cd ..
 
+      # Run the results comparison
+      - name: Compare results
+        if: github.event_name == 'pull_request_target'
+        id: compare
+        shell: bash
+        run: |
+          cd boa
+          echo "::set-output name=comment::$(./target/release/boa_tester compare ../gh-pages/test262/refs/heads/master/latest.json ../results/test262/pull/latest.json -m)"
+
+      - name: Get the PR number
+        if: github.event_name == 'pull_request_target'
+        id: pr-number
+        uses: kkak10/[email protected]
+
+      - name: Find Previous Comment
+        if: github.event_name == 'pull_request_target'
+        uses: peter-evans/find-comment@v1
+        id: previous-comment
+        with:
+          issue-number: ${{ steps.pr-number.outputs.pr }}
+          comment-author: actions-user
+          body-includes: Test262 conformance changes
+
+      - name: Update comment
+        if: github.event_name == 'pull_request_target' && steps.previous-comment.outputs.comment-id
+        uses: peter-evans/create-or-update-comment@v1
+        with:
+          comment-id: ${{ steps.previous-comment.outputs.comment-id }}
+          body: ${{ steps.compare.outputs.comment }}
+
+      - name: Write a new comment
+        if: github.event_name == 'pull_request_target' && !steps.previous-comment.outputs.comment-id
+        uses: peter-evans/create-or-update-comment@v1
+        with:
+          issue-number: ${{ steps.pr-number.outputs.pr }}
+          body: ${{ steps.compare.outputs.comment }}
+
+      # Commit changes to GitHub pages.
       - name: Commit files
         if: github.event_name == 'push'
         run: |
+          cp ./results/test262/* ./gh-pages/test262/
           cd gh-pages
           git config --local user.email "[email protected]"
           git config --local user.name "GitHub Action"
-          git pull --ff-only
           git add test262
           git commit -m "Add new test262 results" -a
           cd ..

CONTRIBUTING.md

@@ -72,7 +72,7 @@ suite, you can just run the normal `cargo test`, and to run the full ECMAScript
 with this command:
 
 ```
-cargo run --release --bin boa_tester -- -v 2> error.log
+cargo run --release --bin boa_tester -- run -v 2> error.log
 ```
 
 Note that this requires the `test262` submodule to be checked out, so you will need to run the following first:
@@ -81,9 +81,15 @@ Note that this requires the `test262` submodule to be checked out, so you will n
 git submodule init && git submodule update
 ```
 
-This will run the test suite in verbose mode (you can remove the `-- -v` part to run it in non-verbose mode),
+This will run the test suite in verbose mode (you can remove the `-v` part to run it in non-verbose mode),
 and output nice colorings in the terminal. It will also output any panic information into the `error.log` file.
 
+You can get some more verbose information that tells you the exact name of each test that is being run, useful
+for debugging purposes by setting up the verbose flag twice, for example `-vv`.
+
+Finally, if you want to only run one sub-suite or even one test (to just check if you fixed/broke something specific),
+you can do it with the `-s` parameter, and then passing the path to the sub-suite or test that you want to run.
+
 ## Communication
 
 We have a Discord server, feel free to ask questions here:

boa/benches/exec.rs

@@ -21,7 +21,9 @@ fn symbol_creation(c: &mut Criterion) {
     let mut engine = Context::new();
 
     // Parse the AST nodes.
-    let nodes = Parser::new(SYMBOL_CREATION.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(SYMBOL_CREATION.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     // Execute the parsed nodes, passing them through a black box, to avoid over-optimizing by the compiler
     c.bench_function("Symbols (Execution)", move |b| {
@@ -36,7 +38,7 @@ fn for_loop_execution(c: &mut Criterion) {
     let mut engine = Context::new();
 
     // Parse the AST nodes.
-    let nodes = Parser::new(FOR_LOOP.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(FOR_LOOP.as_bytes(), false).parse_all().unwrap();
 
     // Execute the parsed nodes, passing them through a black box, to avoid over-optimizing by the compiler
     c.bench_function("For loop (Execution)", move |b| {
@@ -51,7 +53,9 @@ fn fibonacci(c: &mut Criterion) {
     let mut engine = Context::new();
 
     // Parse the AST nodes.
-    let nodes = Parser::new(FIBONACCI.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(FIBONACCI.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     // Execute the parsed nodes, passing them through a black box, to avoid over-optimizing by the compiler
     c.bench_function("Fibonacci (Execution)", move |b| {
@@ -66,7 +70,9 @@ fn object_creation(c: &mut Criterion) {
     let mut engine = Context::new();
 
     // Parse the AST nodes.
-    let nodes = Parser::new(OBJECT_CREATION.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(OBJECT_CREATION.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     // Execute the parsed nodes, passing them through a black box, to avoid over-optimizing by the compiler
     c.bench_function("Object Creation (Execution)", move |b| {
@@ -81,7 +87,7 @@ fn object_prop_access_const(c: &mut Criterion) {
     let mut engine = Context::new();
 
     // Parse the AST nodes.
-    let nodes = Parser::new(OBJECT_PROP_ACCESS_CONST.as_bytes())
+    let nodes = Parser::new(OBJECT_PROP_ACCESS_CONST.as_bytes(), false)
         .parse_all()
         .unwrap();
 
@@ -98,7 +104,7 @@ fn object_prop_access_dyn(c: &mut Criterion) {
     let mut engine = Context::new();
 
     // Parse the AST nodes.
-    let nodes = Parser::new(OBJECT_PROP_ACCESS_DYN.as_bytes())
+    let nodes = Parser::new(OBJECT_PROP_ACCESS_DYN.as_bytes(), false)
         .parse_all()
         .unwrap();
 
@@ -115,7 +121,7 @@ fn regexp_literal_creation(c: &mut Criterion) {
     let mut engine = Context::new();
 
     // Parse the AST nodes.
-    let nodes = Parser::new(REGEXP_LITERAL_CREATION.as_bytes())
+    let nodes = Parser::new(REGEXP_LITERAL_CREATION.as_bytes(), false)
         .parse_all()
         .unwrap();
 
@@ -132,7 +138,9 @@ fn regexp_creation(c: &mut Criterion) {
     let mut engine = Context::new();
 
     // Parse the AST nodes.
-    let nodes = Parser::new(REGEXP_CREATION.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(REGEXP_CREATION.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     // Execute the parsed nodes, passing them through a black box, to avoid over-optimizing by the compiler
     c.bench_function("RegExp (Execution)", move |b| {
@@ -147,7 +155,9 @@ fn regexp_literal(c: &mut Criterion) {
     let mut engine = Context::new();
 
     // Parse the AST nodes.
-    let nodes = Parser::new(REGEXP_LITERAL.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(REGEXP_LITERAL.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     // Execute the parsed nodes, passing them through a black box, to avoid over-optimizing by the compiler
     c.bench_function("RegExp Literal (Execution)", move |b| {
@@ -162,7 +172,7 @@ fn regexp(c: &mut Criterion) {
     let mut engine = Context::new();
 
     // Parse the AST nodes.
-    let nodes = Parser::new(REGEXP.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(REGEXP.as_bytes(), false).parse_all().unwrap();
 
     // Execute the parsed nodes, passing them through a black box, to avoid over-optimizing by the compiler
     c.bench_function("RegExp (Execution)", move |b| {
@@ -175,7 +185,9 @@ static ARRAY_ACCESS: &str = include_str!("bench_scripts/array_access.js");
 fn array_access(c: &mut Criterion) {
     let mut engine = Context::new();
 
-    let nodes = Parser::new(ARRAY_ACCESS.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(ARRAY_ACCESS.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     c.bench_function("Array access (Execution)", move |b| {
         b.iter(|| black_box(&nodes).run(&mut engine).unwrap())
@@ -187,7 +199,9 @@ static ARRAY_CREATE: &str = include_str!("bench_scripts/array_create.js");
 fn array_creation(c: &mut Criterion) {
     let mut engine = Context::new();
 
-    let nodes = Parser::new(ARRAY_CREATE.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(ARRAY_CREATE.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     c.bench_function("Array creation (Execution)", move |b| {
         b.iter(|| black_box(&nodes).run(&mut engine).unwrap())
@@ -199,7 +213,9 @@ static ARRAY_POP: &str = include_str!("bench_scripts/array_pop.js");
 fn array_pop(c: &mut Criterion) {
     let mut engine = Context::new();
 
-    let nodes = Parser::new(ARRAY_POP.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(ARRAY_POP.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     c.bench_function("Array pop (Execution)", move |b| {
         b.iter(|| black_box(&nodes).run(&mut engine).unwrap())
@@ -211,7 +227,9 @@ static STRING_CONCAT: &str = include_str!("bench_scripts/string_concat.js");
 fn string_concat(c: &mut Criterion) {
     let mut engine = Context::new();
 
-    let nodes = Parser::new(STRING_CONCAT.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(STRING_CONCAT.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     c.bench_function("String concatenation (Execution)", move |b| {
         b.iter(|| black_box(&nodes).run(&mut engine).unwrap())
@@ -223,7 +241,9 @@ static STRING_COMPARE: &str = include_str!("bench_scripts/string_compare.js");
 fn string_compare(c: &mut Criterion) {
     let mut engine = Context::new();
 
-    let nodes = Parser::new(STRING_COMPARE.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(STRING_COMPARE.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     c.bench_function("String comparison (Execution)", move |b| {
         b.iter(|| black_box(&nodes).run(&mut engine).unwrap())
@@ -235,7 +255,9 @@ static STRING_COPY: &str = include_str!("bench_scripts/string_copy.js");
 fn string_copy(c: &mut Criterion) {
     let mut engine = Context::new();
 
-    let nodes = Parser::new(STRING_COPY.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(STRING_COPY.as_bytes(), false)
+        .parse_all()
+        .unwrap();
 
     c.bench_function("String copy (Execution)", move |b| {
         b.iter(|| black_box(&nodes).run(&mut engine).unwrap())
@@ -247,7 +269,7 @@ static NUMBER_OBJECT_ACCESS: &str = include_str!("bench_scripts/number_object_ac
 fn number_object_access(c: &mut Criterion) {
     let mut engine = Context::new();
 
-    let nodes = Parser::new(NUMBER_OBJECT_ACCESS.as_bytes())
+    let nodes = Parser::new(NUMBER_OBJECT_ACCESS.as_bytes(), false)
         .parse_all()
         .unwrap();
 
@@ -261,7 +283,7 @@ static BOOLEAN_OBJECT_ACCESS: &str = include_str!("bench_scripts/boolean_object_
 fn boolean_object_access(c: &mut Criterion) {
     let mut engine = Context::new();
 
-    let nodes = Parser::new(BOOLEAN_OBJECT_ACCESS.as_bytes())
+    let nodes = Parser::new(BOOLEAN_OBJECT_ACCESS.as_bytes(), false)
         .parse_all()
         .unwrap();
 
@@ -275,7 +297,7 @@ static STRING_OBJECT_ACCESS: &str = include_str!("bench_scripts/string_object_ac
 fn string_object_access(c: &mut Criterion) {
     let mut engine = Context::new();
 
-    let nodes = Parser::new(STRING_OBJECT_ACCESS.as_bytes())
+    let nodes = Parser::new(STRING_OBJECT_ACCESS.as_bytes(), false)
         .parse_all()
         .unwrap();
 
@@ -289,7 +311,7 @@ static ARITHMETIC_OPERATIONS: &str = include_str!("bench_scripts/arithmetic_oper
 fn arithmetic_operations(c: &mut Criterion) {
     let mut engine = Context::new();
 
-    let nodes = Parser::new(ARITHMETIC_OPERATIONS.as_bytes())
+    let nodes = Parser::new(ARITHMETIC_OPERATIONS.as_bytes(), false)
         .parse_all()
         .unwrap();
 
@@ -302,7 +324,7 @@ static CLEAN_JS: &str = include_str!("bench_scripts/clean_js.js");
 
 fn clean_js(c: &mut Criterion) {
     let mut engine = Context::new();
-    let nodes = Parser::new(CLEAN_JS.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(CLEAN_JS.as_bytes(), false).parse_all().unwrap();
     c.bench_function("Clean js (Execution)", move |b| {
         b.iter(|| black_box(&nodes).run(&mut engine).unwrap())
     });
@@ -312,7 +334,7 @@ static MINI_JS: &str = include_str!("bench_scripts/mini_js.js");
 
 fn mini_js(c: &mut Criterion) {
     let mut engine = Context::new();
-    let nodes = Parser::new(MINI_JS.as_bytes()).parse_all().unwrap();
+    let nodes = Parser::new(MINI_JS.as_bytes(), false).parse_all().unwrap();
     c.bench_function("Mini js (Execution)", move |b| {
         b.iter(|| black_box(&nodes).run(&mut engine).unwrap())
     });