From bd3e08fbb790270d9c8d30541a9e444dd696baa3 Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Mon, 24 May 2021 20:52:20 +0200 Subject: [PATCH 01/11] Added some result changes for Test262 result comparisons --- boa_tester/src/results.rs | 150 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/boa_tester/src/results.rs b/boa_tester/src/results.rs index b1901993cbb..bbaab3082c4 100644 --- a/boa_tester/src/results.rs +++ b/boa_tester/src/results.rs @@ -202,6 +202,8 @@ pub(crate) fn compare_results(base: &Path, new: &Path, markdown: bool) { let new_conformance = (new_passed as f64 / new_total as f64) * 100_f64; let conformance_diff = new_conformance - base_conformance; + let test_diff = compute_result_diff(base, &base_results.results, &new_results.results); + if markdown { use num_format::{Locale, ToFormattedString}; @@ -269,6 +271,46 @@ pub(crate) fn compare_results(base: &Path, new: &Path, markdown: bool) { }, ), ); + + if !test_diff.fixed.is_empty() { + println!(); + println!("#### Fixed tests:"); + println!("```"); + for test in test_diff.fixed { + println!("{}", test); + } + println!("```"); + } + + if !test_diff.broken.is_empty() { + println!(); + println!("#### Broken tests:"); + println!("```"); + for test in test_diff.broken { + println!("{}", test); + } + println!("```"); + } + + if !test_diff.new_panics.is_empty() { + println!(); + println!("#### New panics:"); + println!("```"); + for test in test_diff.new_panics { + println!("{}", test); + } + println!("```"); + } + + if !test_diff.panic_fixes.is_empty() { + println!(); + println!("#### Fixed panics:"); + println!("```"); + for test in test_diff.panic_fixes { + println!("{}", test); + } + println!("```"); + } } else { println!("Test262 conformance changes:"); println!("| Test result | master | PR | difference |"); @@ -296,5 +338,113 @@ pub(crate) fn compare_results(base: &Path, new: &Path, markdown: bool) { new_panics, base_panics - new_panics ); + + if !test_diff.fixed.is_empty() { + println!(); + println!("Fixed tests:"); + for test in test_diff.fixed { + println!("{}", test); + } + } + + if !test_diff.broken.is_empty() { + println!(); + println!("Broken tests:"); + for test in test_diff.broken { + println!("{}", test); + } + } + + if !test_diff.new_panics.is_empty() { + println!(); + println!("New panics:"); + for test in test_diff.new_panics { + println!("{}", test); + } + } + + if !test_diff.panic_fixes.is_empty() { + println!(); + println!("Fixed panics:"); + for test in test_diff.panic_fixes { + println!("{}", test); + } + } + } +} + +/// Test differences. +#[derive(Debug, Clone, Default)] +struct ResultDiff { + fixed: Vec>, + broken: Vec>, + new_panics: Vec>, + panic_fixes: Vec>, +} + +impl ResultDiff { + /// Extends the diff with new results. + fn extend(&mut self, new: Self) { + self.fixed.extend(new.fixed); + self.broken.extend(new.broken); + self.new_panics.extend(new.new_panics); + self.panic_fixes.extend(new.panic_fixes); + } +} + +/// Compares a base and a new result and returns the list of differences. +fn compute_result_diff( + base: &Path, + base_result: &SuiteResult, + new_result: &SuiteResult, +) -> ResultDiff { + use super::TestOutcomeResult; + + let mut final_diff = ResultDiff::default(); + + for base_test in &base_result.tests { + if let Some(new_test) = new_result + .tests + .iter() + .find(|new_test| new_test.name == base_test.name) + { + let test_name = format!( + "{}/{}.js (previously {:?})", + base.display(), + new_test.name, + base_test.result + ) + .into_boxed_str(); + + match (base_test.result, new_test.result) { + (TestOutcomeResult::Passed, TestOutcomeResult::Passed) + | (TestOutcomeResult::Ignored, TestOutcomeResult::Ignored) + | (TestOutcomeResult::Failed, TestOutcomeResult::Failed) + | (TestOutcomeResult::Panic, TestOutcomeResult::Panic) => {} + + (_, TestOutcomeResult::Passed) => final_diff.fixed.push(test_name), + (_, TestOutcomeResult::Failed) => final_diff.broken.push(test_name), + (_, TestOutcomeResult::Panic) => final_diff.new_panics.push(test_name), + (TestOutcomeResult::Panic, _) => final_diff.panic_fixes.push(test_name), + _ => {} + } + + todo!("check difference"); + } + } + + for base_suite in &base_result.suites { + if let Some(new_suite) = new_result + .suites + .iter() + .find(|new_suite| new_suite.name == base_suite.name) + { + let new_base = base.join(new_suite.name.as_ref()); + let diff = compute_result_diff(new_base.as_path(), base_suite, new_suite); + + final_diff.extend(diff) + } } + + final_diff } From 5f571eb00813d233446c91113bc31a7566a50147 Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Mon, 24 May 2021 20:58:09 +0200 Subject: [PATCH 02/11] Added a bug and a panic to test the changes in the comparator --- boa/src/syntax/ast/node/field/get_const_field/mod.rs | 3 ++- boa/src/syntax/ast/node/new/mod.rs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/boa/src/syntax/ast/node/field/get_const_field/mod.rs b/boa/src/syntax/ast/node/field/get_const_field/mod.rs index c5b1de5dbe7..44bf30e8232 100644 --- a/boa/src/syntax/ast/node/field/get_const_field/mod.rs +++ b/boa/src/syntax/ast/node/field/get_const_field/mod.rs @@ -69,7 +69,8 @@ impl Executable for GetConstField { obj = Value::Object(obj.to_object(context)?); } - obj.get_field(self.field(), context) + // obj.get_field(self.field(), context) + Ok(Value::undefined()) } } diff --git a/boa/src/syntax/ast/node/new/mod.rs b/boa/src/syntax/ast/node/new/mod.rs index 85be684e413..609d9f9e2cf 100644 --- a/boa/src/syntax/ast/node/new/mod.rs +++ b/boa/src/syntax/ast/node/new/mod.rs @@ -62,6 +62,7 @@ impl Executable for New { let next_value = next.value(); v_args.push(next_value.clone()); } + panic!(); break; // after spread we don't accept any new arguments } else { v_args.push(arg.run(context)?); From 0641c366d2c99a592874b689305c07095c2eab66 Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Mon, 24 May 2021 21:49:17 +0200 Subject: [PATCH 03/11] Remove explicit todo!() --- boa_tester/src/results.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/boa_tester/src/results.rs b/boa_tester/src/results.rs index bbaab3082c4..0e0a99e5910 100644 --- a/boa_tester/src/results.rs +++ b/boa_tester/src/results.rs @@ -428,8 +428,6 @@ fn compute_result_diff( (TestOutcomeResult::Panic, _) => final_diff.panic_fixes.push(test_name), _ => {} } - - todo!("check difference"); } } From e95b09d805affc8da306a73e00191ac4499ce08f Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Mon, 24 May 2021 22:01:12 +0200 Subject: [PATCH 04/11] Fixing output for test links --- boa_tester/src/results.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/boa_tester/src/results.rs b/boa_tester/src/results.rs index 0e0a99e5910..7bcee7405f2 100644 --- a/boa_tester/src/results.rs +++ b/boa_tester/src/results.rs @@ -409,8 +409,10 @@ fn compute_result_diff( .find(|new_test| new_test.name == base_test.name) { let test_name = format!( - "{}/{}.js (previously {:?})", - base.display(), + "test262/test/{}/{}.js (previously {:?})", + base.strip_prefix("../gh-pages/test262/refs/heads/master/latest.json") + .expect("error removing prefix") + .display(), new_test.name, base_test.result ) From ccac1e0f4a338240c1a81de10c2434cfddc6efdb Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Mon, 24 May 2021 22:02:03 +0200 Subject: [PATCH 05/11] Force comment --- .github/workflows/test262.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test262.yml b/.github/workflows/test262.yml index 3290f1c06f2..ba4536e3444 100644 --- a/.github/workflows/test262.yml +++ b/.github/workflows/test262.yml @@ -75,7 +75,7 @@ jobs: body-includes: Test262 conformance changes - name: Update comment - if: github.event_name == 'pull_request' && steps.previous-comment.outputs.comment-id + # if: github.event_name == 'pull_request' && steps.previous-comment.outputs.comment-id uses: peter-evans/create-or-update-comment@v1.4.5 continue-on-error: true with: @@ -84,7 +84,7 @@ jobs: edit-mode: replace - name: Write a new comment - if: github.event_name == 'pull_request' && !steps.previous-comment.outputs.comment-id + # if: github.event_name == 'pull_request' && !steps.previous-comment.outputs.comment-id uses: peter-evans/create-or-update-comment@v1.4.5 continue-on-error: true with: From 6936f9517d7c793c2c69b47ce2a3feb48362262f Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Mon, 24 May 2021 22:10:52 +0200 Subject: [PATCH 06/11] Reducing failures so that commenting works --- boa/src/syntax/ast/node/field/get_const_field/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/boa/src/syntax/ast/node/field/get_const_field/mod.rs b/boa/src/syntax/ast/node/field/get_const_field/mod.rs index 44bf30e8232..c5b1de5dbe7 100644 --- a/boa/src/syntax/ast/node/field/get_const_field/mod.rs +++ b/boa/src/syntax/ast/node/field/get_const_field/mod.rs @@ -69,8 +69,7 @@ impl Executable for GetConstField { obj = Value::Object(obj.to_object(context)?); } - // obj.get_field(self.field(), context) - Ok(Value::undefined()) + obj.get_field(self.field(), context) } } From dec8b96cd1e413af282fc7e9a0fec27049df4d14 Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Mon, 24 May 2021 22:17:53 +0200 Subject: [PATCH 07/11] New test --- boa/src/builtins/bigint/mod.rs | 2 +- boa/src/syntax/ast/node/new/mod.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/boa/src/builtins/bigint/mod.rs b/boa/src/builtins/bigint/mod.rs index 89fc30dd032..ee4406c2541 100644 --- a/boa/src/builtins/bigint/mod.rs +++ b/boa/src/builtins/bigint/mod.rs @@ -42,7 +42,7 @@ mod tests; pub struct BigInt(num_bigint::BigInt); impl BuiltIn for BigInt { - const NAME: &'static str = "BigInt"; + const NAME: &'static str = "BigInt2"; fn attribute() -> Attribute { Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE diff --git a/boa/src/syntax/ast/node/new/mod.rs b/boa/src/syntax/ast/node/new/mod.rs index 609d9f9e2cf..85be684e413 100644 --- a/boa/src/syntax/ast/node/new/mod.rs +++ b/boa/src/syntax/ast/node/new/mod.rs @@ -62,7 +62,6 @@ impl Executable for New { let next_value = next.value(); v_args.push(next_value.clone()); } - panic!(); break; // after spread we don't accept any new arguments } else { v_args.push(arg.run(context)?); From fa529b3c5f8ba7263cac914f6fcacfb41f92aab8 Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Mon, 24 May 2021 22:29:58 +0200 Subject: [PATCH 08/11] Fixed a bit of formatting --- boa_tester/src/results.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/boa_tester/src/results.rs b/boa_tester/src/results.rs index 7bcee7405f2..0a4033ae491 100644 --- a/boa_tester/src/results.rs +++ b/boa_tester/src/results.rs @@ -406,14 +406,19 @@ fn compute_result_diff( if let Some(new_test) = new_result .tests .iter() - .find(|new_test| new_test.name == base_test.name) + .find(|new_test| new_test.strict == base_test.strict && new_test.name == base_test.name) { let test_name = format!( - "test262/test/{}/{}.js (previously {:?})", + "test/{}/{}.js {}(previously {:?})", base.strip_prefix("../gh-pages/test262/refs/heads/master/latest.json") .expect("error removing prefix") .display(), new_test.name, + if base_test.strict { + "[strict mode] " + } else { + "" + }, base_test.result ) .into_boxed_str(); From a80f9c4f13513fa00d2f267c8c64e1223503459a Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Mon, 24 May 2021 22:36:01 +0200 Subject: [PATCH 09/11] Reverted purposedly introduced bugs --- .github/workflows/test262.yml | 4 ++-- boa/src/builtins/bigint/mod.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test262.yml b/.github/workflows/test262.yml index ba4536e3444..3290f1c06f2 100644 --- a/.github/workflows/test262.yml +++ b/.github/workflows/test262.yml @@ -75,7 +75,7 @@ jobs: body-includes: Test262 conformance changes - name: Update comment - # if: github.event_name == 'pull_request' && steps.previous-comment.outputs.comment-id + if: github.event_name == 'pull_request' && steps.previous-comment.outputs.comment-id uses: peter-evans/create-or-update-comment@v1.4.5 continue-on-error: true with: @@ -84,7 +84,7 @@ jobs: edit-mode: replace - name: Write a new comment - # if: github.event_name == 'pull_request' && !steps.previous-comment.outputs.comment-id + if: github.event_name == 'pull_request' && !steps.previous-comment.outputs.comment-id uses: peter-evans/create-or-update-comment@v1.4.5 continue-on-error: true with: diff --git a/boa/src/builtins/bigint/mod.rs b/boa/src/builtins/bigint/mod.rs index ee4406c2541..89fc30dd032 100644 --- a/boa/src/builtins/bigint/mod.rs +++ b/boa/src/builtins/bigint/mod.rs @@ -42,7 +42,7 @@ mod tests; pub struct BigInt(num_bigint::BigInt); impl BuiltIn for BigInt { - const NAME: &'static str = "BigInt2"; + const NAME: &'static str = "BigInt"; fn attribute() -> Attribute { Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE From 27166e8e1367ccf0f050a265a178e54d2325fba5 Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Tue, 25 May 2021 19:24:26 +0200 Subject: [PATCH 10/11] Added a "details" section for the test262 test diff --- boa_tester/src/results.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/boa_tester/src/results.rs b/boa_tester/src/results.rs index 0a4033ae491..6f48b04c72d 100644 --- a/boa_tester/src/results.rs +++ b/boa_tester/src/results.rs @@ -274,42 +274,46 @@ pub(crate) fn compare_results(base: &Path, new: &Path, markdown: bool) { if !test_diff.fixed.is_empty() { println!(); - println!("#### Fixed tests:"); + println!("
**Fixed tests:**"); println!("```"); for test in test_diff.fixed { println!("{}", test); } println!("```"); + println!("
"); } if !test_diff.broken.is_empty() { println!(); - println!("#### Broken tests:"); + println!("
**Broken tests:**"); println!("```"); for test in test_diff.broken { println!("{}", test); } println!("```"); + println!("
"); } if !test_diff.new_panics.is_empty() { println!(); - println!("#### New panics:"); + println!("
**New panics:**"); println!("```"); for test in test_diff.new_panics { println!("{}", test); } println!("```"); + println!("
"); } if !test_diff.panic_fixes.is_empty() { println!(); - println!("#### Fixed panics:"); + println!("
**Fixed panics:**"); println!("```"); for test in test_diff.panic_fixes { println!("{}", test); } println!("```"); + println!("
"); } } else { println!("Test262 conformance changes:"); From 1ec13589306bc85d9d44aa60a8672d8e1eee8c07 Mon Sep 17 00:00:00 2001 From: "Iban Eguia (Razican)" Date: Fri, 28 May 2021 21:49:28 +0200 Subject: [PATCH 11/11] Fix bold --- boa_tester/src/results.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/boa_tester/src/results.rs b/boa_tester/src/results.rs index 6f48b04c72d..fd6edee77d3 100644 --- a/boa_tester/src/results.rs +++ b/boa_tester/src/results.rs @@ -274,7 +274,7 @@ pub(crate) fn compare_results(base: &Path, new: &Path, markdown: bool) { if !test_diff.fixed.is_empty() { println!(); - println!("
**Fixed tests:**"); + println!("
Fixed tests:"); println!("```"); for test in test_diff.fixed { println!("{}", test); @@ -285,7 +285,7 @@ pub(crate) fn compare_results(base: &Path, new: &Path, markdown: bool) { if !test_diff.broken.is_empty() { println!(); - println!("
**Broken tests:**"); + println!("
Broken tests:"); println!("```"); for test in test_diff.broken { println!("{}", test); @@ -296,7 +296,7 @@ pub(crate) fn compare_results(base: &Path, new: &Path, markdown: bool) { if !test_diff.new_panics.is_empty() { println!(); - println!("
**New panics:**"); + println!("
New panics:"); println!("```"); for test in test_diff.new_panics { println!("{}", test); @@ -307,7 +307,7 @@ pub(crate) fn compare_results(base: &Path, new: &Path, markdown: bool) { if !test_diff.panic_fixes.is_empty() { println!(); - println!("
**Fixed panics:**"); + println!("
Fixed panics:"); println!("```"); for test in test_diff.panic_fixes { println!("{}", test);