compiletest: Run revisions as independent tests.

ehuss · ehuss · commit e3d8fc50444a · 2018-05-16T22:25:20.000-07:00
- The output of each test is now in its own directory.
- "auxiliary" output is now under the respective test directory.
- `stage_id` removed from filenames, and instead placed in the stamp file as a hash.  This helps keep path lengths down for Windows.

In brief, the new layout looks like this:
```
&lt;build_base&gt;/&lt;relative_dir&gt;/&lt;testname&gt;.&lt;revision&gt;.&lt;mode&gt;/
    stamp
    &lt;testname&gt;.err
    &lt;testname&gt;.out
    a (binary)
    auxiliary/lib&lt;auxname&gt;.dylib
    auxiliary/&lt;auxname&gt;/&lt;auxname&gt;.err
    auxiliary/&lt;auxname&gt;/&lt;auxname&gt;.out
```
(revision and mode are optional)
diff --git a/src/test/ui-fulldeps/update-references.sh b/src/test/ui-fulldeps/update-references.sh
@@ -31,18 +31,17 @@ MYDIR=$(dirname $0)
 BUILD_DIR="$1"
 shift
 
+shopt -s nullglob
+
 while [[ "$1" != "" ]]; do
-    STDERR_NAME="${1/%.rs/.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
 done
diff --git a/src/test/ui/update-references.sh b/src/test/ui/update-references.sh
@@ -35,7 +35,7 @@ shopt -s nullglob
 
 while [[ "$1" != "" ]]; do
     for EXT in "stderr" "stdout" "fixed"; do
-        for OUT_NAME in $BUILD_DIR/${1%.rs}.*$EXT; do
+        for OUT_NAME in $BUILD_DIR/${1%.rs}*/*$EXT; do
             OUT_DIR=`dirname "$1"`
             OUT_BASE=`basename "$OUT_NAME"`
             if ! (diff $OUT_NAME $MYDIR/$OUT_DIR/$OUT_BASE >& /dev/null); then
diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs
@@ -10,10 +10,11 @@
 pub use self::Mode::*;
 
 use std::fmt;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
 use test::ColorConfig;
+use util::PathBufExt;
 
 #[derive(Clone, Copy, PartialEq, Debug)]
 pub enum Mode {
@@ -245,10 +246,9 @@ pub struct Config {
     pub nodejs: Option<String>,
 }
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct TestPaths {
     pub file: PathBuf,         // e.g., compile-test/foo/bar/baz.rs
-    pub base: PathBuf,         // e.g., compile-test, auxiliary
     pub relative_dir: PathBuf, // e.g., foo/bar
 }
 
@@ -278,3 +278,38 @@ pub const UI_EXTENSIONS: &[&str] = &[UI_STDERR, UI_STDOUT, UI_FIXED];
 pub const UI_STDERR: &str = "stderr";
 pub const UI_STDOUT: &str = "stdout";
 pub const UI_FIXED: &str = "fixed";
+
+/// Absolute path to the directory where all output for all tests in the given
+/// `relative_dir` group should reside. Example:
+///   /path/to/build/host-triple/test/ui/relative/
+/// This is created early when tests are collected to avoid race conditions.
+pub fn output_relative_path(config: &Config, relative_dir: &Path) -> PathBuf {
+    config.build_base.join(relative_dir)
+}
+
+/// Generates a unique name for the test, such as `testname.revision.mode`.
+pub fn output_testname_unique(
+    config: &Config,
+    testpaths: &TestPaths,
+    revision: Option<&str>,
+) -> PathBuf {
+    let mode = config.compare_mode.as_ref().map_or("", |m| m.to_str());
+    PathBuf::from(&testpaths.file.file_stem().unwrap())
+        .with_extra_extension(revision.unwrap_or(""))
+        .with_extra_extension(mode)
+}
+
+/// Absolute path to the directory where all output for the given
+/// test/revision should reside.  Example:
+///   /path/to/build/host-triple/test/ui/relative/testname.revision.mode/
+pub fn output_base_dir(config: &Config, testpaths: &TestPaths, revision: Option<&str>) -> PathBuf {
+    output_relative_path(config, &testpaths.relative_dir)
+        .join(output_testname_unique(config, testpaths, revision))
+}
+
+/// Absolute path to the base filename used as output for the given
+/// test/revision.  Example:
+///   /path/to/build/host-triple/test/ui/relative/testname.revision.mode/testname
+pub fn output_base_name(config: &Config, testpaths: &TestPaths, revision: Option<&str>) -> PathBuf {
+    output_base_dir(config, testpaths, revision).join(testpaths.file.file_stem().unwrap())
+}
diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs
@@ -32,15 +32,15 @@ extern crate test;
 extern crate rustfix;
 
 use common::CompareMode;
-use common::{expected_output_path, UI_EXTENSIONS};
+use common::{expected_output_path, output_base_dir, output_relative_path, UI_EXTENSIONS};
 use common::{Config, TestPaths};
 use common::{DebugInfoGdb, DebugInfoLldb, Mode, Pretty};
 use filetime::FileTime;
 use getopts::Options;
 use std::env;
 use std::ffi::OsString;
 use std::fs;
-use std::io;
+use std::io::{self, Read};
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use test::ColorConfig;
@@ -552,10 +552,9 @@ fn collect_tests_from_dir(
         if name == *"Makefile" && config.mode == Mode::RunMake {
             let paths = TestPaths {
                 file: dir.to_path_buf(),
-                base: base.to_path_buf(),
                 relative_dir: relative_dir_path.parent().unwrap().to_path_buf(),
             };
-            tests.push(make_test(config, &paths));
+            tests.extend(make_test(config, &paths));
             return Ok(());
         }
     }
@@ -566,7 +565,7 @@ fn collect_tests_from_dir(
     // sequential loop because otherwise, if we do it in the
     // tests themselves, they race for the privilege of
     // creating the directories and sometimes fail randomly.
-    let build_dir = config.build_base.join(&relative_dir_path);
+    let build_dir = output_relative_path(config, relative_dir_path);
     fs::create_dir_all(&build_dir).unwrap();
 
     // Add each `.rs` file as a test, and recurse further on any
@@ -580,21 +579,12 @@ fn collect_tests_from_dir(
             debug!("found test file: {:?}", file_path.display());
             let paths = TestPaths {
                 file: file_path,
-                base: base.to_path_buf(),
                 relative_dir: relative_dir_path.to_path_buf(),
             };
-            tests.push(make_test(config, &paths))
+            tests.extend(make_test(config, &paths))
         } else if file_path.is_dir() {
             let relative_file_path = relative_dir_path.join(file.file_name());
-            if &file_name == "auxiliary" {
-                // `aux` directories contain other crates used for
-                // cross-crate tests. Don't search them for tests, but
-                // do create a directory in the build dir for them,
-                // since we will dump intermediate output in there
-                // sometimes.
-                let build_dir = config.build_base.join(&relative_file_path);
-                fs::create_dir_all(&build_dir).unwrap();
-            } else {
+            if &file_name != "auxiliary" {
                 debug!("found directory: {:?}", file_path.display());
                 collect_tests_from_dir(config, base, &file_path, &relative_file_path, tests)?;
             }
@@ -617,7 +607,7 @@ pub fn is_test(file_name: &OsString) -> bool {
     !invalid_prefixes.iter().any(|p| file_name.starts_with(p))
 }
 
-pub fn make_test(config: &Config, testpaths: &TestPaths) -> test::TestDescAndFn {
+pub fn make_test(config: &Config, testpaths: &TestPaths) -> Vec<test::TestDescAndFn> {
     let early_props = if config.mode == Mode::RunMake {
         // Allow `ignore` directives to be in the Makefile.
         EarlyProps::from_file(config, &testpaths.file.join("Makefile"))
@@ -637,46 +627,68 @@ pub fn make_test(config: &Config, testpaths: &TestPaths) -> test::TestDescAndFn
         },
     };
 
-    // Debugging emscripten code doesn't make sense today
-    let ignore = early_props.ignore || !up_to_date(config, testpaths, &early_props)
-        || (config.mode == DebugInfoGdb || config.mode == DebugInfoLldb)
-            && config.target.contains("emscripten");
-
-    test::TestDescAndFn {
-        desc: test::TestDesc {
-            name: make_test_name(config, testpaths),
-            ignore,
-            should_panic,
-            allow_fail: false,
-        },
-        testfn: make_test_closure(config, testpaths),
-    }
+    // Incremental tests are special, they inherently cannot be run in parallel.
+    // `runtest::run` will be responsible for iterating over revisions.
+    let revisions = if early_props.revisions.is_empty() || config.mode == Mode::Incremental {
+        vec![None]
+    } else {
+        early_props.revisions.iter().map(|r| Some(r)).collect()
+    };
+    revisions
+        .into_iter()
+        .map(|revision| {
+            // Debugging emscripten code doesn't make sense today
+            let ignore = early_props.ignore
+                || !up_to_date(
+                    config,
+                    testpaths,
+                    &early_props,
+                    revision.map(|s| s.as_str()),
+                )
+                || (config.mode == DebugInfoGdb || config.mode == DebugInfoLldb)
+                    && config.target.contains("emscripten");
+            test::TestDescAndFn {
+                desc: test::TestDesc {
+                    name: make_test_name(config, testpaths, revision),
+                    ignore,
+                    should_panic,
+                    allow_fail: false,
+                },
+                testfn: make_test_closure(config, testpaths, revision),
+            }
+        })
+        .collect()
 }
 
-fn stamp(config: &Config, testpaths: &TestPaths) -> PathBuf {
-    let mode_suffix = match config.compare_mode {
-        Some(ref mode) => format!("-{}", mode.to_str()),
-        None => format!(""),
-    };
-    let stamp_name = format!(
-        "{}-{}{}.stamp",
-        testpaths.file.file_name().unwrap().to_str().unwrap(),
-        config.stage_id,
-        mode_suffix
-    );
-    config
-        .build_base
-        .canonicalize()
-        .unwrap_or_else(|_| config.build_base.clone())
-        .join(&testpaths.relative_dir)
-        .join(stamp_name)
+fn stamp(config: &Config, testpaths: &TestPaths, revision: Option<&str>) -> PathBuf {
+    output_base_dir(config, testpaths, revision).join("stamp")
 }
 
-fn up_to_date(config: &Config, testpaths: &TestPaths, props: &EarlyProps) -> bool {
+fn up_to_date(
+    config: &Config,
+    testpaths: &TestPaths,
+    props: &EarlyProps,
+    revision: Option<&str>,
+) -> bool {
+    let stamp_name = stamp(config, testpaths, revision);
+    // Check hash.
+    let mut f = match fs::File::open(&stamp_name) {
+        Ok(f) => f,
+        Err(_) => return true,
+    };
+    let mut contents = String::new();
+    f.read_to_string(&mut contents)
+        .expect("Can't read stamp contents");
+    let expected_hash = runtest::compute_stamp_hash(config);
+    if contents != expected_hash {
+        return true;
+    }
+
+    // Check timestamps.
     let rust_src_dir = config
         .find_rust_src_root()
         .expect("Could not find Rust source root");
-    let stamp = mtime(&stamp(config, testpaths));
+    let stamp = mtime(&stamp_name);
     let mut inputs = vec![mtime(&testpaths.file), mtime(&config.rustc_path)];
     for aux in props.aux.iter() {
         inputs.push(mtime(&testpaths
@@ -714,16 +726,8 @@ fn up_to_date(config: &Config, testpaths: &TestPaths, props: &EarlyProps) -> boo
 
     // UI test files.
     for extension in UI_EXTENSIONS {
-        for revision in &props.revisions {
-            let path =
-                &expected_output_path(testpaths, Some(revision), &config.compare_mode, extension);
-            inputs.push(mtime(path));
-        }
-
-        if props.revisions.is_empty() {
-            let path = &expected_output_path(testpaths, None, &config.compare_mode, extension);
-            inputs.push(mtime(path));
-        }
+        let path = &expected_output_path(testpaths, revision, &config.compare_mode, extension);
+        inputs.push(mtime(path));
     }
 
     inputs.iter().any(|input| *input > stamp)
@@ -735,7 +739,11 @@ fn mtime(path: &Path) -> FileTime {
         .unwrap_or_else(|_| FileTime::zero())
 }
 
-pub fn make_test_name(config: &Config, testpaths: &TestPaths) -> test::TestName {
+fn make_test_name(
+    config: &Config,
+    testpaths: &TestPaths,
+    revision: Option<&String>,
+) -> test::TestName {
     // Convert a complete path to something like
     //
     //    run-pass/foo/bar/baz.rs
@@ -747,17 +755,25 @@ pub fn make_test_name(config: &Config, testpaths: &TestPaths) -> test::TestName
         None => format!(""),
     };
     test::DynTestName(format!(
-        "[{}{}] {}",
+        "[{}{}] {}{}",
         config.mode,
         mode_suffix,
-        path.display()
+        path.display(),
+        revision.map_or("".to_string(), |rev| format!("#{}", rev))
     ))
 }
 
-pub fn make_test_closure(config: &Config, testpaths: &TestPaths) -> test::TestFn {
+fn make_test_closure(
+    config: &Config,
+    testpaths: &TestPaths,
+    revision: Option<&String>,
+) -> test::TestFn {
     let config = config.clone();
     let testpaths = testpaths.clone();
-    test::DynTestFn(Box::new(move || runtest::run(config, &testpaths)))
+    let revision = revision.cloned();
+    test::DynTestFn(Box::new(move || {
+        runtest::run(config, &testpaths, revision.as_ref().map(|s| s.as_str()))
+    }))
 }
 
 /// Returns (Path to GDB, GDB Version, GDB has Rust Support)
diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs
diff --git a/src/tools/compiletest/src/util.rs b/src/tools/compiletest/src/util.rs
