From cb501a6fe6cdb1928611b7193dec96f3d7bd2293 Mon Sep 17 00:00:00 2001 From: Nicholas-Baron Date: Sat, 2 Oct 2021 14:25:53 -0700 Subject: [PATCH 1/3] Move items related to computing diffs to a separate file --- src/tools/compiletest/src/compute_diff.rs | 106 +++++++++++++++++++++ src/tools/compiletest/src/main.rs | 1 + src/tools/compiletest/src/runtest.rs | 108 +--------------------- 3 files changed, 109 insertions(+), 106 deletions(-) create mode 100644 src/tools/compiletest/src/compute_diff.rs diff --git a/src/tools/compiletest/src/compute_diff.rs b/src/tools/compiletest/src/compute_diff.rs new file mode 100644 index 0000000000000..9ca0c69dbd3e3 --- /dev/null +++ b/src/tools/compiletest/src/compute_diff.rs @@ -0,0 +1,106 @@ +use std::collections::VecDeque; + +#[derive(Debug, PartialEq)] +pub enum DiffLine { + Context(String), + Expected(String), + Resulting(String), +} + +#[derive(Debug, PartialEq)] +pub struct Mismatch { + pub line_number: u32, + pub lines: Vec, +} + +impl Mismatch { + fn new(line_number: u32) -> Mismatch { + Mismatch { line_number, lines: Vec::new() } + } +} + +// Produces a diff between the expected output and actual output. +pub fn make_diff(expected: &str, actual: &str, context_size: usize) -> Vec { + let mut line_number = 1; + let mut context_queue: VecDeque<&str> = VecDeque::with_capacity(context_size); + let mut lines_since_mismatch = context_size + 1; + let mut results = Vec::new(); + let mut mismatch = Mismatch::new(0); + + for result in diff::lines(expected, actual) { + match result { + diff::Result::Left(str) => { + if lines_since_mismatch >= context_size && lines_since_mismatch > 0 { + results.push(mismatch); + mismatch = Mismatch::new(line_number - context_queue.len() as u32); + } + + while let Some(line) = context_queue.pop_front() { + mismatch.lines.push(DiffLine::Context(line.to_owned())); + } + + mismatch.lines.push(DiffLine::Expected(str.to_owned())); + line_number += 1; + lines_since_mismatch = 0; + } + diff::Result::Right(str) => { + if lines_since_mismatch >= context_size && lines_since_mismatch > 0 { + results.push(mismatch); + mismatch = Mismatch::new(line_number - context_queue.len() as u32); + } + + while let Some(line) = context_queue.pop_front() { + mismatch.lines.push(DiffLine::Context(line.to_owned())); + } + + mismatch.lines.push(DiffLine::Resulting(str.to_owned())); + lines_since_mismatch = 0; + } + diff::Result::Both(str, _) => { + if context_queue.len() >= context_size { + let _ = context_queue.pop_front(); + } + + if lines_since_mismatch < context_size { + mismatch.lines.push(DiffLine::Context(str.to_owned())); + } else if context_size > 0 { + context_queue.push_back(str); + } + + line_number += 1; + lines_since_mismatch += 1; + } + } + } + + results.push(mismatch); + results.remove(0); + + results +} + +pub(crate) fn write_diff(expected: &str, actual: &str, context_size: usize) -> String { + use std::fmt::Write; + let mut output = String::new(); + let diff_results = make_diff(expected, actual, context_size); + for result in diff_results { + let mut line_number = result.line_number; + for line in result.lines { + match line { + DiffLine::Expected(e) => { + writeln!(output, "-\t{}", e).unwrap(); + line_number += 1; + } + DiffLine::Context(c) => { + writeln!(output, "{}\t{}", line_number, c).unwrap(); + line_number += 1; + } + DiffLine::Resulting(r) => { + writeln!(output, "+\t{}", r).unwrap(); + } + } + } + writeln!(output).unwrap(); + } + output +} diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 9e655557fd271..fbf3249db94d1 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -28,6 +28,7 @@ use self::header::{make_test_description, EarlyProps}; mod tests; pub mod common; +pub mod compute_diff; pub mod errors; pub mod header; mod json; diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 2a4bb9eb88b30..ec6c745b7d202 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -8,6 +8,7 @@ use crate::common::{CompareMode, FailMode, PassMode}; use crate::common::{Config, TestPaths}; use crate::common::{Pretty, RunPassValgrind}; use crate::common::{UI_RUN_STDERR, UI_RUN_STDOUT}; +use crate::compute_diff::write_diff; use crate::errors::{self, Error, ErrorKind}; use crate::header::TestProps; use crate::json; @@ -18,7 +19,7 @@ use regex::{Captures, Regex}; use rustfix::{apply_suggestions, get_suggestions_from_json, Filter}; use std::collections::hash_map::DefaultHasher; -use std::collections::{HashMap, HashSet, VecDeque}; +use std::collections::{HashMap, HashSet}; use std::env; use std::ffi::{OsStr, OsString}; use std::fs::{self, create_dir_all, File, OpenOptions}; @@ -100,111 +101,6 @@ pub fn get_lib_name(lib: &str, dylib: bool) -> String { } } -#[derive(Debug, PartialEq)] -pub enum DiffLine { - Context(String), - Expected(String), - Resulting(String), -} - -#[derive(Debug, PartialEq)] -pub struct Mismatch { - pub line_number: u32, - pub lines: Vec, -} - -impl Mismatch { - fn new(line_number: u32) -> Mismatch { - Mismatch { line_number, lines: Vec::new() } - } -} - -// Produces a diff between the expected output and actual output. -pub fn make_diff(expected: &str, actual: &str, context_size: usize) -> Vec { - let mut line_number = 1; - let mut context_queue: VecDeque<&str> = VecDeque::with_capacity(context_size); - let mut lines_since_mismatch = context_size + 1; - let mut results = Vec::new(); - let mut mismatch = Mismatch::new(0); - - for result in diff::lines(expected, actual) { - match result { - diff::Result::Left(str) => { - if lines_since_mismatch >= context_size && lines_since_mismatch > 0 { - results.push(mismatch); - mismatch = Mismatch::new(line_number - context_queue.len() as u32); - } - - while let Some(line) = context_queue.pop_front() { - mismatch.lines.push(DiffLine::Context(line.to_owned())); - } - - mismatch.lines.push(DiffLine::Expected(str.to_owned())); - line_number += 1; - lines_since_mismatch = 0; - } - diff::Result::Right(str) => { - if lines_since_mismatch >= context_size && lines_since_mismatch > 0 { - results.push(mismatch); - mismatch = Mismatch::new(line_number - context_queue.len() as u32); - } - - while let Some(line) = context_queue.pop_front() { - mismatch.lines.push(DiffLine::Context(line.to_owned())); - } - - mismatch.lines.push(DiffLine::Resulting(str.to_owned())); - lines_since_mismatch = 0; - } - diff::Result::Both(str, _) => { - if context_queue.len() >= context_size { - let _ = context_queue.pop_front(); - } - - if lines_since_mismatch < context_size { - mismatch.lines.push(DiffLine::Context(str.to_owned())); - } else if context_size > 0 { - context_queue.push_back(str); - } - - line_number += 1; - lines_since_mismatch += 1; - } - } - } - - results.push(mismatch); - results.remove(0); - - results -} - -fn write_diff(expected: &str, actual: &str, context_size: usize) -> String { - use std::fmt::Write; - let mut output = String::new(); - let diff_results = make_diff(expected, actual, context_size); - for result in diff_results { - let mut line_number = result.line_number; - for line in result.lines { - match line { - DiffLine::Expected(e) => { - writeln!(output, "-\t{}", e).unwrap(); - line_number += 1; - } - DiffLine::Context(c) => { - writeln!(output, "{}\t{}", line_number, c).unwrap(); - line_number += 1; - } - DiffLine::Resulting(r) => { - writeln!(output, "+\t{}", r).unwrap(); - } - } - } - writeln!(output).unwrap(); - } - output -} - pub fn run(config: Config, testpaths: &TestPaths, revision: Option<&str>) { match &*config.target { "arm-linux-androideabi" From 2a57a462498e3ce954e0b5b7ddd2502bd74875f4 Mon Sep 17 00:00:00 2001 From: Nicholas-Baron Date: Sun, 3 Oct 2021 16:53:45 -0700 Subject: [PATCH 2/3] Extract a portion of diff writing code to separate function --- src/tools/compiletest/src/compute_diff.rs | 47 +++++++++++++++++++++++ src/tools/compiletest/src/runtest.rs | 41 ++------------------ 2 files changed, 50 insertions(+), 38 deletions(-) diff --git a/src/tools/compiletest/src/compute_diff.rs b/src/tools/compiletest/src/compute_diff.rs index 9ca0c69dbd3e3..fb837a6077a7f 100644 --- a/src/tools/compiletest/src/compute_diff.rs +++ b/src/tools/compiletest/src/compute_diff.rs @@ -1,4 +1,5 @@ use std::collections::VecDeque; +use std::path::Path; #[derive(Debug, PartialEq)] pub enum DiffLine { @@ -104,3 +105,49 @@ pub(crate) fn write_diff(expected: &str, actual: &str, context_size: usize) -> S } output } + +/// Returns whether any data was actually written. +pub(crate) fn write_rustdoc_diff( + diff_filename: &str, + out_dir: &Path, + compare_dir: &Path, + verbose: bool, +) -> bool { + use std::fs::File; + use std::io::{Read, Write}; + let mut diff_output = File::create(diff_filename).unwrap(); + let mut wrote_data = false; + for entry in walkdir::WalkDir::new(out_dir) { + let entry = entry.expect("failed to read file"); + let extension = entry.path().extension().and_then(|p| p.to_str()); + if entry.file_type().is_file() + && (extension == Some("html".into()) || extension == Some("js".into())) + { + let expected_path = compare_dir.join(entry.path().strip_prefix(&out_dir).unwrap()); + let expected = if let Ok(s) = std::fs::read(&expected_path) { s } else { continue }; + let actual_path = entry.path(); + let actual = std::fs::read(&actual_path).unwrap(); + let diff = unified_diff::diff( + &expected, + &expected_path.to_string_lossy(), + &actual, + &actual_path.to_string_lossy(), + 3, + ); + wrote_data |= !diff.is_empty(); + diff_output.write_all(&diff).unwrap(); + } + } + + if !wrote_data { + println!("note: diff is identical to nightly rustdoc"); + assert!(diff_output.metadata().unwrap().len() == 0); + return false; + } else if verbose { + eprintln!("printing diff:"); + let mut buf = Vec::new(); + diff_output.read_to_end(&mut buf).unwrap(); + std::io::stderr().lock().write_all(&mut buf).unwrap(); + } + true +} diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index ec6c745b7d202..ffe8f1bb56dfe 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -8,7 +8,7 @@ use crate::common::{CompareMode, FailMode, PassMode}; use crate::common::{Config, TestPaths}; use crate::common::{Pretty, RunPassValgrind}; use crate::common::{UI_RUN_STDERR, UI_RUN_STDOUT}; -use crate::compute_diff::write_diff; +use crate::compute_diff::{write_diff, write_rustdoc_diff}; use crate::errors::{self, Error, ErrorKind}; use crate::header::TestProps; use crate::json; @@ -2403,43 +2403,8 @@ impl<'test> TestCx<'test> { let diff_filename = format!("build/tmp/rustdoc-compare-{}.diff", std::process::id()); - { - let mut diff_output = File::create(&diff_filename).unwrap(); - let mut wrote_data = false; - for entry in walkdir::WalkDir::new(out_dir) { - let entry = entry.expect("failed to read file"); - let extension = entry.path().extension().and_then(|p| p.to_str()); - if entry.file_type().is_file() - && (extension == Some("html".into()) || extension == Some("js".into())) - { - let expected_path = - compare_dir.join(entry.path().strip_prefix(&out_dir).unwrap()); - let expected = - if let Ok(s) = std::fs::read(&expected_path) { s } else { continue }; - let actual_path = entry.path(); - let actual = std::fs::read(&actual_path).unwrap(); - let diff = unified_diff::diff( - &expected, - &expected_path.to_string_lossy(), - &actual, - &actual_path.to_string_lossy(), - 3, - ); - wrote_data |= !diff.is_empty(); - diff_output.write_all(&diff).unwrap(); - } - } - - if !wrote_data { - println!("note: diff is identical to nightly rustdoc"); - assert!(diff_output.metadata().unwrap().len() == 0); - return; - } else if self.config.verbose { - eprintln!("printing diff:"); - let mut buf = Vec::new(); - diff_output.read_to_end(&mut buf).unwrap(); - std::io::stderr().lock().write_all(&mut buf).unwrap(); - } + if !write_rustdoc_diff(&diff_filename, out_dir, &compare_dir, self.config.verbose) { + return; } match self.config.color { From 3760c91252b1a6697fbd05e98e72c7e78493a72e Mon Sep 17 00:00:00 2001 From: Nicholas-Baron Date: Sun, 3 Oct 2021 17:07:43 -0700 Subject: [PATCH 3/3] Make write_rustdoc_diff a more generic function --- src/tools/compiletest/src/compute_diff.rs | 16 ++++++++++------ src/tools/compiletest/src/runtest.rs | 13 +++++++++++-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/tools/compiletest/src/compute_diff.rs b/src/tools/compiletest/src/compute_diff.rs index fb837a6077a7f..92c80c27de03b 100644 --- a/src/tools/compiletest/src/compute_diff.rs +++ b/src/tools/compiletest/src/compute_diff.rs @@ -1,4 +1,5 @@ use std::collections::VecDeque; +use std::fs::{File, FileType}; use std::path::Path; #[derive(Debug, PartialEq)] @@ -106,23 +107,26 @@ pub(crate) fn write_diff(expected: &str, actual: &str, context_size: usize) -> S output } +/// Filters based on filetype and extension whether to diff a file. +/// /// Returns whether any data was actually written. -pub(crate) fn write_rustdoc_diff( +pub(crate) fn write_filtered_diff( diff_filename: &str, out_dir: &Path, compare_dir: &Path, verbose: bool, -) -> bool { - use std::fs::File; + filter: Filter, +) -> bool +where + Filter: Fn(FileType, Option<&str>) -> bool, +{ use std::io::{Read, Write}; let mut diff_output = File::create(diff_filename).unwrap(); let mut wrote_data = false; for entry in walkdir::WalkDir::new(out_dir) { let entry = entry.expect("failed to read file"); let extension = entry.path().extension().and_then(|p| p.to_str()); - if entry.file_type().is_file() - && (extension == Some("html".into()) || extension == Some("js".into())) - { + if filter(entry.file_type(), extension) { let expected_path = compare_dir.join(entry.path().strip_prefix(&out_dir).unwrap()); let expected = if let Ok(s) = std::fs::read(&expected_path) { s } else { continue }; let actual_path = entry.path(); diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index ffe8f1bb56dfe..0821e279d2485 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -8,7 +8,7 @@ use crate::common::{CompareMode, FailMode, PassMode}; use crate::common::{Config, TestPaths}; use crate::common::{Pretty, RunPassValgrind}; use crate::common::{UI_RUN_STDERR, UI_RUN_STDOUT}; -use crate::compute_diff::{write_diff, write_rustdoc_diff}; +use crate::compute_diff::{write_diff, write_filtered_diff}; use crate::errors::{self, Error, ErrorKind}; use crate::header::TestProps; use crate::json; @@ -2403,7 +2403,16 @@ impl<'test> TestCx<'test> { let diff_filename = format!("build/tmp/rustdoc-compare-{}.diff", std::process::id()); - if !write_rustdoc_diff(&diff_filename, out_dir, &compare_dir, self.config.verbose) { + if !write_filtered_diff( + &diff_filename, + out_dir, + &compare_dir, + self.config.verbose, + |file_type, extension| { + file_type.is_file() + && (extension == Some("html".into()) || extension == Some("js".into())) + }, + ) { return; }