ci: comment compare

.github/workflows/test262.yml

@@ -18,7 +18,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [windows-latest, macos-latest, ubuntu-latest]
+       os: [windows-latest, macos-latest, ubuntu-latest]
     steps:
       - name: Checkout repository
         uses: actions/checkout@v2
@@ -33,31 +33,28 @@ jobs:
         uses: Swatinem/rust-cache@v1
       - name: Run Test262 suite
         continue-on-error: true
-        run: cargo xtask coverage --json
-      - name: Rename the emitted file
-        run: |
-          mv base_results.json new_results.json
+        run:  cargo xtask coverage --json > new_results.json
       - name: Save test results
         uses: actions/upload-artifact@v2
         with:
           name: new_results
           path: new_results.json
 
-  compare:
+# This job compares the coverage results from this PR and the coverage results from master
+  coverage-comparison:
     needs: coverage
     name: Compare coverage
     runs-on: ${{ matrix.os }}
     strategy:
       fail-fast: false
       matrix:
-        os: [windows-latest, macos-latest, ubuntu-latest]
+       os: [windows-latest, macos-latest, ubuntu-latest]
     steps:
       - name: Checkout repository
         uses: actions/checkout@v2
         with:
           submodules: recursive
-          # To restore once the main branch has the new command
-          # ref: main
+          ref: main
       - name: Install toolchain
         uses: actions-rs/toolchain@v1
         with:
@@ -67,10 +64,55 @@ jobs:
         uses: Swatinem/rust-cache@v1
       - name: Run Test262 suite on main branch
         continue-on-error: true
-        run: cargo xtask coverage --json
+        run:  cargo xtask coverage --json > base_results.json
       - name: Download PRs test results
         uses: actions/download-artifact@v2
         with:
           name: new_results
       - name: Compare results on ${{ matrix.os }}
-        run: cargo xtask compare ./base_results.json ./new_results.json  --markdown
+        if: github.event_name == 'pull_request'
+        id: comparison
+        shell: bash
+        run: |
+          comment="$(cargo xtask compare ./base_results.json ./new_results.json  --markdown)"
+          comment="${comment//'%'/'%25'}"
+          comment="${comment//$'\n'/'%0A'}"
+          comment="${comment//$'\r'/'%0D'}"
+          echo "::set-output name=comment::$comment"
+
+      - name: Get the PR number
+        if: github.event_name == 'pull_request'
+        id: pr-number
+        uses: kkak10/pr-number-action@v1.3
+
+      - name: Find Previous Comment
+        if: github.event_name == 'pull_request'
+        uses: peter-evans/find-comment@v1.3.0
+        id: previous-comment
+        with:
+          issue-number: ${{ steps.pr-number.outputs.pr }}
+          body-includes: Test262 conformance changes
+
+      - name: Update comment
+        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:
+          comment-id: ${{ steps.previous-comment.outputs.comment-id }}
+          body: |
+            ### Test262 comparison coverage results on ${{ matrix.os }}
+
+            ${{ steps.comparison.outputs.comment }}
+          edit-mode: replace
+
+      - name: Write a new comment
+        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:
+          issue-number: ${{ steps.pr-number.outputs.pr }}
+          body: |
+            ### Test262 comparison coverage results on ${{ matrix.os }}
+
+            ${{ steps.comparison.outputs.comment }}
+

crates/rslint_parser/src/parser.rs

@@ -423,19 +423,24 @@ impl<'t> Parser<'t> {
 	}
 
 	/// Bump and add an error event
-	pub fn err_and_bump(&mut self, err: impl Into<ParserError>) {
+	pub fn err_and_bump(&mut self, err: impl Into<ParserError>, unknown_syntax_kind: SyntaxKind) {
 		let m = self.start();
 		self.bump_any();
-		m.complete(self, SyntaxKind::ERROR);
+		m.complete(self, unknown_syntax_kind);
 		self.error(err);
 	}
 
-	pub fn err_if_not_ts(&mut self, mut marker: impl BorrowMut<CompletedMarker>, err: &str) {
+	pub fn err_if_not_ts(
+		&mut self,
+		mut marker: impl BorrowMut<CompletedMarker>,
+		err: &str,
+		unknown_syntax_kind: SyntaxKind,
+	) {
 		if self.typescript() {
 			return;
 		}
 		let borrow = marker.borrow_mut();
-		borrow.change_kind(self, SyntaxKind::ERROR);
+		borrow.change_kind(self, unknown_syntax_kind);
 		let err = self.err_builder(err).primary(borrow.range(self), "");
 
 		self.error(err);
@@ -489,7 +494,11 @@ impl<'t> Parser<'t> {
 		start..end
 	}
 
-	pub fn expr_with_semi_recovery(&mut self, assign: bool) -> Option<CompletedMarker> {
+	pub fn expr_with_semi_recovery(
+		&mut self,
+		assign: bool,
+		unknown_syntax_kind: SyntaxKind,
+	) -> Option<CompletedMarker> {
 		let func = if assign {
 			syntax::expr::assign_expr
 		} else {
@@ -504,7 +513,7 @@ impl<'t> Parser<'t> {
 
 			self.error(err);
 			self.bump_any();
-			m.complete(self, SyntaxKind::ERROR);
+			m.complete(self, unknown_syntax_kind);
 			return None;
 		}
 
@@ -715,7 +724,7 @@ impl CompletedMarker {
 	}
 
 	pub fn err_if_not_ts(&mut self, p: &mut Parser, err: &str) {
-		p.err_if_not_ts(self, err);
+		p.err_if_not_ts(self, err, SyntaxKind::ERROR);
 	}
 }
 

crates/rslint_parser/src/syntax/expr.rs

@@ -1026,7 +1026,7 @@ pub fn primary_expr(p: &mut Parser) -> Option<CompletedMarker> {
 						))
 						.primary(p.cur_tok().range, "");
 
-					p.err_and_bump(err);
+					p.err_and_bump(err, ERROR);
 					m.complete(p, ERROR)
 				} else {
 					let err = p

crates/rslint_parser/src/syntax/stmt.rs

@@ -158,7 +158,7 @@ pub fn stmt(
 
 			// We must explicitly handle this case or else infinite recursion can happen
 			if p.at_ts(token_set![T!['}'], T![import], T![export]]) {
-				p.err_and_bump(err);
+				p.err_and_bump(err, ERROR);
 				return None;
 			}
 
@@ -231,7 +231,7 @@ fn expr_stmt(p: &mut Parser, decorator: Option<CompletedMarker>) -> Option<Compl
 		m.complete(p, ERROR);
 	}
 
-	let mut expr = p.expr_with_semi_recovery(false)?;
+	let mut expr = p.expr_with_semi_recovery(false, ERROR)?;
 	// Labelled stmt
 	if expr.kind() == NAME_REF && p.at(T![:]) {
 		expr.change_kind(p, NAME);
@@ -315,7 +315,7 @@ pub fn throw_stmt(p: &mut Parser) -> CompletedMarker {
 
 		p.error(err);
 	} else {
-		p.expr_with_semi_recovery(false);
+		p.expr_with_semi_recovery(false, ERROR);
 	}
 	semi(p, start..p.cur_tok().range.end);
 	m.complete(p, JS_THROW_STATEMENT)
@@ -407,7 +407,7 @@ pub fn return_stmt(p: &mut Parser) -> CompletedMarker {
 	let start = p.cur_tok().range.start;
 	p.expect(T![return]);
 	if !p.has_linebreak_before_n(0) && p.at_ts(STARTS_EXPR) {
-		p.expr_with_semi_recovery(false);
+		p.expr_with_semi_recovery(false, ERROR);
 	}
 	semi(p, start..p.cur_tok().range.end);
 	let complete = m.complete(p, JS_RETURN_STATEMENT);
@@ -852,13 +852,13 @@ fn variable_initializer(p: &mut Parser) {
 	let m = p.start();
 
 	p.expect(T![=]);
-	p.expr_with_semi_recovery(true);
+	p.expr_with_semi_recovery(true, ERROR);
 
 	m.complete(p, SyntaxKind::JS_EQUAL_VALUE_CLAUSE);
 }
 
 // A do.. while statement, such as `do {} while (true)`
-// test do_while_stmt
+// test do_while_stmtR
 // do { } while (true)
 // do throw Error("foo") while (true)
 pub fn do_stmt(p: &mut Parser) -> CompletedMarker {

xtask/src/compare/mod.rs

@@ -3,7 +3,11 @@ use std::path::PathBuf;
 
 mod results;
 
-pub fn run(base_result_path: Option<&str>, new_result_path: Option<&str>, markdown: bool) {
+pub fn coverage_compare(
+	base_result_path: Option<&str>,
+	new_result_path: Option<&str>,
+	markdown: bool,
+) {
 	// resolve the path passed as argument, or retrieve the default one
 	let base_result_dir = if let Some(base_result_path) = base_result_path {
 		PathBuf::from(base_result_path)

xtask/src/compare/results.rs

@@ -1,7 +1,7 @@
-use crate::coverage::files::{Outcome, TestResults};
+use crate::coverage::files::{Outcome, TestResult, TestResults};
 use ascii_table::{AsciiTable, Column};
 use colored::Colorize;
-use std::{fs::File, path::Path};
+use std::{collections::HashMap, ffi::OsStr, fs::File, path::Path};
 
 pub fn emit_compare(base: &Path, new: &Path, markdown: bool) {
 	let base_results: TestResults =
@@ -35,9 +35,37 @@ pub fn emit_compare(base: &Path, new: &Path, markdown: bool) {
 
 	if markdown {
 		/// Generates a proper diff format, with some bold text if things change.
-		fn diff_format(diff: isize) -> String {
+		fn diff_format(diff: isize, i_am_passed_results: bool, show_increase: bool) -> String {
+			let good = "✅ ";
+			let bad = "❌ ";
+			let up = "⏫ ";
+			let down = "⏬ ";
+
+			let emoji = if show_increase {
+				match diff.cmp(&0) {
+					std::cmp::Ordering::Less => {
+						if i_am_passed_results {
+							format!("{}{}", bad, down)
+						} else {
+							format!("{}{}", good, down)
+						}
+					}
+					std::cmp::Ordering::Equal => format!(""),
+					std::cmp::Ordering::Greater => {
+						if i_am_passed_results {
+							format!("{}{}", good, up)
+						} else {
+							format!("{}{}", bad, up)
+						}
+					}
+				}
+			} else {
+				format!("")
+			};
+
 			format!(
-				"{}{}{}{}",
+				"{}{}{}{}{}",
+				emoji,
 				if diff != 0 { "**" } else { "" },
 				if diff > 0 { "+" } else { "" },
 				diff,
@@ -52,28 +80,28 @@ pub fn emit_compare(base: &Path, new: &Path, markdown: bool) {
 			"| Total | {} | {} | {} |",
 			base_total,
 			new_total,
-			diff_format(total_diff)
+			diff_format(total_diff, false, false)
 		);
 
 		println!(
 			"| Passed | {} | {} | {} |",
 			base_passed,
 			new_passed,
-			diff_format(passed_diff)
+			diff_format(passed_diff, true, true)
 		);
 
 		println!(
 			"| Failed | {} | {} | {} |",
 			base_failed,
 			new_failed,
-			diff_format(failed_diff)
+			diff_format(failed_diff, false, true)
 		);
 
 		println!(
 			"| Panics | {} | {} | {} |",
-			base_passed,
-			new_passed,
-			diff_format(panics_diff)
+			base_panics,
+			new_panics,
+			diff_format(panics_diff, false, true)
 		);
 
 		println!(
@@ -209,11 +237,13 @@ fn compare_diffs<'a>(
 	new_results: &'a TestResults,
 ) -> ReportDiff<'a> {
 	let mut report_diff = ReportDiff::new();
+	let new_paths: HashMap<&OsStr, &TestResult> = new_results
+		.details
+		.iter()
+		.map(|detail| return (detail.path.as_os_str(), detail))
+		.collect();
 	for base_result in &base_results.details {
-		let test_to_analyze = new_results
-			.details
-			.iter()
-			.find(|new_test| new_test.path.as_os_str().eq(base_result.path.as_os_str()));
+		let test_to_analyze = new_paths.get(base_result.path.as_os_str());
 
 		if let Some(test_to_analyze) = test_to_analyze {
 			match (&base_result.outcome, &test_to_analyze.outcome) {