Rollup merge of rust-lang#49812 - ehuss:ui-test-revisions, r=nikomatsakis

Fix revision support for UI tests.

Fixes rust-lang#48878

Author: kennytm

src/test/ui/update-references.sh

@@ -31,24 +31,17 @@ MYDIR=$(dirname $0)
 BUILD_DIR="$1"
 shift
 
+shopt -s nullglob
+
 while [[ "$1" != "" ]]; do
-    STDERR_NAME="${1/%.rs/.stderr}"
-    STDERR_NLL_NAME="${1/%.rs/.nll.stderr}"
-    STDOUT_NAME="${1/%.rs/.stdout}"
+    for EXT in "stderr" "stdout"; do
+        for OUT_NAME in $BUILD_DIR/${1%.rs}.*$EXT; do
+            OUT_BASE=`basename "$OUT_NAME"`
+            if ! (diff $OUT_NAME $MYDIR/$OUT_BASE >& /dev/null); then
+                echo updating $MYDIR/$OUT_BASE
+                cp $OUT_NAME $MYDIR
+            fi
+        done
+    done
     shift
-    if [ -f $BUILD_DIR/$STDOUT_NAME ] && \
-           ! (diff $BUILD_DIR/$STDOUT_NAME $MYDIR/$STDOUT_NAME >& /dev/null); then
-        echo updating $MYDIR/$STDOUT_NAME
-        cp $BUILD_DIR/$STDOUT_NAME $MYDIR/$STDOUT_NAME
-    fi
-    if [ -f $BUILD_DIR/$STDERR_NAME ] && \
-           ! (diff $BUILD_DIR/$STDERR_NAME $MYDIR/$STDERR_NAME >& /dev/null); then
-        echo updating $MYDIR/$STDERR_NAME
-        cp $BUILD_DIR/$STDERR_NAME $MYDIR/$STDERR_NAME
-    fi
-    if [ -f $BUILD_DIR/$STDERR_NLL_NAME ] && \
-           ! (diff $BUILD_DIR/$STDERR_NLL_NAME $MYDIR/$STDERR_NLL_NAME >& /dev/null); then
-        echo updating $MYDIR/$STDERR_NLL_NAME
-        cp $BUILD_DIR/$STDERR_NLL_NAME $MYDIR/$STDERR_NLL_NAME
-    fi
 done

src/tools/compiletest/src/runtest.rs

@@ -26,7 +26,7 @@ use std::collections::VecDeque;
 use std::collections::HashMap;
 use std::collections::HashSet;
 use std::env;
-use std::ffi::OsString;
+use std::ffi::{OsStr, OsString};
 use std::fs::{self, create_dir_all, File};
 use std::fmt;
 use std::io::prelude::*;
@@ -72,6 +72,26 @@ impl Mismatch {
     }
 }
 
+trait PathBufExt {
+    /// Append an extension to the path, even if it already has one.
+    fn with_extra_extension<S: AsRef<OsStr>>(&self, extension: S) -> PathBuf;
+}
+
+impl PathBufExt for PathBuf {
+    fn with_extra_extension<S: AsRef<OsStr>>(&self, extension: S) -> PathBuf {
+        if extension.as_ref().len() == 0 {
+            self.clone()
+        } else {
+            let mut fname = self.file_name().unwrap().to_os_string();
+            if !extension.as_ref().to_str().unwrap().starts_with(".") {
+                fname.push(".");
+            }
+            fname.push(extension);
+            self.with_file_name(fname)
+        }
+    }
+}
+
 // Produces a diff between the expected output and actual output.
 pub fn make_diff(expected: &str, actual: &str, context_size: usize) -> Vec<Mismatch> {
     let mut line_number = 1;
@@ -1725,20 +1745,14 @@ impl<'test> TestCx<'test> {
     }
 
     fn make_exe_name(&self) -> PathBuf {
-        let mut f = self.output_base_name();
+        let mut f = self.output_base_name_stage();
         // FIXME: This is using the host architecture exe suffix, not target!
         if self.config.target.contains("emscripten") {
-            let mut fname = f.file_name().unwrap().to_os_string();
-            fname.push(".js");
-            f.set_file_name(&fname);
+            f = f.with_extra_extension("js");
         } else if self.config.target.contains("wasm32") {
-            let mut fname = f.file_name().unwrap().to_os_string();
-            fname.push(".wasm");
-            f.set_file_name(&fname);
+            f = f.with_extra_extension("wasm");
         } else if !env::consts::EXE_SUFFIX.is_empty() {
-            let mut fname = f.file_name().unwrap().to_os_string();
-            fname.push(env::consts::EXE_SUFFIX);
-            f.set_file_name(&fname);
+            f = f.with_extra_extension(env::consts::EXE_SUFFIX);
         }
         f
     }
@@ -1846,25 +1860,28 @@ impl<'test> TestCx<'test> {
     }
 
     fn aux_output_dir_name(&self) -> PathBuf {
-        let f = self.output_base_name();
-        let mut fname = f.file_name().unwrap().to_os_string();
-        fname.push(&format!("{}.aux", self.config.mode.disambiguator()));
-        f.with_file_name(&fname)
+        self.output_base_name_stage()
+            .with_extra_extension(self.config.mode.disambiguator())
+            .with_extra_extension(".aux")
     }
 
     fn output_testname(&self, filepath: &Path) -> PathBuf {
         PathBuf::from(filepath.file_stem().unwrap())
     }
 
-    /// Given a test path like `compile-fail/foo/bar.rs` Returns a name like
-    ///
-    ///     <output>/foo/bar-stage1
+    /// Given a test path like `compile-fail/foo/bar.rs` returns a name like
+    /// `/path/to/build/<triple>/test/compile-fail/foo/bar`.
     fn output_base_name(&self) -> PathBuf {
         let dir = self.config.build_base.join(&self.testpaths.relative_dir);
 
         // Note: The directory `dir` is created during `collect_tests_from_dir`
         dir.join(&self.output_testname(&self.testpaths.file))
-            .with_extension(&self.config.stage_id)
+    }
+
+    /// Same as `output_base_name`, but includes the stage ID as an extension,
+    /// such as: `.../compile-fail/foo/bar.stage1-<triple>`
+    fn output_base_name_stage(&self) -> PathBuf {
+        self.output_base_name().with_extension(&self.config.stage_id)
     }
 
     fn maybe_dump_to_stdout(&self, out: &str, err: &str) {
@@ -1989,7 +2006,7 @@ impl<'test> TestCx<'test> {
     fn run_rustdoc_test(&self) {
         assert!(self.revision.is_none(), "revisions not relevant here");
 
-        let out_dir = self.output_base_name();
+        let out_dir = self.output_base_name_stage();
         let _ = fs::remove_dir_all(&out_dir);
         create_dir_all(&out_dir).unwrap();
 
@@ -2391,7 +2408,7 @@ impl<'test> TestCx<'test> {
             .unwrap();
         let src_root = cwd.join(&src_root);
 
-        let tmpdir = cwd.join(self.output_base_name());
+        let tmpdir = cwd.join(self.output_base_name_stage());
         if tmpdir.exists() {
             self.aggressive_rm_rf(&tmpdir).unwrap();
         }
@@ -2816,7 +2833,6 @@ impl<'test> TestCx<'test> {
                                             self.revision,
                                             &self.config.compare_mode,
                                             kind);
-
         if !path.exists() && self.config.compare_mode.is_some() {
             // fallback!
             path = expected_output_path(&self.testpaths, self.revision, &None, kind);
@@ -2880,10 +2896,12 @@ impl<'test> TestCx<'test> {
             }
         }
 
-        let expected_output = self.expected_output_path(kind);
-        // #50113: output is abspath; only want filename component.
-        let expected_output = expected_output.file_name().expect("output path requires file name");
-        let output_file = self.output_base_name().with_file_name(&expected_output);
+        let mode = self.config.compare_mode.as_ref().map_or("", |m| m.to_str());
+        let output_file = self.output_base_name()
+            .with_extra_extension(self.revision.unwrap_or(""))
+            .with_extra_extension(mode)
+            .with_extra_extension(kind);
+
         match File::create(&output_file).and_then(|mut f| f.write_all(actual.as_bytes())) {
             Ok(()) => {}
             Err(e) => self.fatal(&format!(

src/tools/tidy/src/ui_tests.rs

@@ -12,38 +12,26 @@
 
 use std::path::Path;
 
-// See rust-lang/rust#48879: In addition to the mapping from `foo.rs`
-// to `foo.stderr`/`foo.stdout`, we also can optionally have
-// `foo.$mode.stderr`, where $mode is one of the strings on this list,
-// as an alternative to use when running under that mode.
-static COMPARE_MODE_NAMES: [&'static str; 1] = ["nll"];
-
 pub fn check(path: &Path, bad: &mut bool) {
     super::walk_many(&[&path.join("test/ui"), &path.join("test/ui-fulldeps")],
                      &mut |_| false,
                      &mut |file_path| {
         if let Some(ext) = file_path.extension() {
-            if (ext == "stderr" || ext == "stdout") && !file_path.with_extension("rs").exists() {
-
-                // rust-lang/rust#48879: this fn used to be beautful
-                // because Path API special-cases replacing
-                // extensions. That works great for ".stderr" but not
-                // so well for ".nll.stderr". To support the latter,
-                // we explicitly search backwards for mode's starting
-                // point and build corresponding source name.
-                let filename = file_path.file_name().expect("need filename")
-                    .to_str().expect("need UTF-8 filename");
-                let found_matching_prefix = COMPARE_MODE_NAMES.iter().any(|mode| {
-                    if let Some(r_idx) = filename.rfind(&format!(".{}", mode)) {
-                        let source_name = format!("{}.rs", &filename[0..r_idx]);
-                        let source_path = file_path.with_file_name(source_name);
-                        source_path.exists()
-                    } else {
-                        false
-                    }
-                });
-
-                if !found_matching_prefix {
+            if ext == "stderr" || ext == "stdout" {
+                // Test output filenames have the format:
+                // $testname.stderr
+                // $testname.$mode.stderr
+                // $testname.$revision.stderr
+                // $testname.$revision.$mode.stderr
+                //
+                // For now, just make sure that there is a corresponding
+                // $testname.rs file.
+                let testname = file_path.file_name().unwrap()
+                                        .to_str().unwrap()
+                                        .splitn(2, '.').next().unwrap();
+                if !file_path.with_file_name(testname)
+                             .with_extension("rs")
+                             .exists() {
                     println!("Stray file with UI testing output: {:?}", file_path);
                     *bad = true;
                 }