Rollup merge of #86156 - ehuss:linkchecker-fixes, r=Mark-Simulacrum

JohnTitor · web-flow · commit e435e3259dc6 · 2021-06-21T09:42:15.000+09:00
Fix a bug in the linkchecker There was a small typo in the linkchecker (in #85652) that caused it to report a `#` fragment link error pointing to the wrong file (it was displaying the path to the source file, not the target of the link). This also includes a few other changes: - Fixes the tests due to some changes in the redirect handling in #84703. - Adds the tests to rustbuild to run whenever the linkchecker itself is run. - Updates the tests to validate more of the output (so that a mistake like this would have been caught). Closes #86144
diff --git a/src/bootstrap/builder/tests.rs b/src/bootstrap/builder/tests.rs
@@ -613,9 +613,14 @@ mod dist {
         // Note that the stages here are +1 than what they actually are because
         // Rustdoc::run swaps out the compiler with stage minus 1 if --stage is
         // not 0.
+        //
+        // The stage 0 copy is the one downloaded for bootstrapping. It is
+        // (currently) needed to run "cargo test" on the linkchecker, and
+        // should be relatively "free".
         assert_eq!(
             first(builder.cache.all::<tool::Rustdoc>()),
             &[
+                tool::Rustdoc { compiler: Compiler { host: a, stage: 0 } },
                 tool::Rustdoc { compiler: Compiler { host: a, stage: 1 } },
                 tool::Rustdoc { compiler: Compiler { host: a, stage: 2 } },
             ]
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
@@ -124,8 +124,25 @@ You can skip linkcheck with --exclude src/tools/linkchecker"
 
         builder.info(&format!("Linkcheck ({})", host));
 
+        // Test the linkchecker itself.
+        let bootstrap_host = builder.config.build;
+        let compiler = builder.compiler(0, bootstrap_host);
+        let cargo = tool::prepare_tool_cargo(
+            builder,
+            compiler,
+            Mode::ToolBootstrap,
+            bootstrap_host,
+            "test",
+            "src/tools/linkchecker",
+            SourceType::InTree,
+            &[],
+        );
+        try_run(builder, &mut cargo.into());
+
+        // Build all the default documentation.
         builder.default_doc(&[]);
 
+        // Run the linkchecker.
         let _time = util::timeit(&builder);
         try_run(
             builder,
diff --git a/src/tools/linkchecker/main.rs b/src/tools/linkchecker/main.rs
@@ -347,7 +347,7 @@ impl Checker {
                 } else {
                     report.errors += 1;
                     print!("{}:{}: broken link fragment ", pretty_path, i + 1);
-                    println!("`#{}` pointing to `{}`", fragment, pretty_path);
+                    println!("`#{}` pointing to `{}`", fragment, target_pretty_path);
                 };
             }
         });
diff --git a/src/tools/linkchecker/tests/broken_redir/redir-bad.html b/src/tools/linkchecker/tests/broken_redir/redir-bad.html
@@ -2,6 +2,7 @@
 <html lang="en">
 <head>
     <meta http-equiv="refresh" content="0;URL=sometarget">
+    <title>Redirection</title>
 </head>
 <body>
     <p>Redirecting to <a href="sometarget">sometarget</a>...</p>
diff --git a/src/tools/linkchecker/tests/checks.rs b/src/tools/linkchecker/tests/checks.rs
@@ -15,7 +15,7 @@ fn run(dirname: &str) -> (ExitStatus, String, String) {
 fn broken_test(dirname: &str, expected: &str) {
     let (status, stdout, stderr) = run(dirname);
     assert!(!status.success());
-    if !stdout.contains(expected) {
+    if !contains(expected, &stdout) {
         panic!(
             "stdout did not contain expected text: {}\n\
             --- stdout:\n\
@@ -27,6 +27,25 @@ fn broken_test(dirname: &str, expected: &str) {
     }
 }
 
+fn contains(expected: &str, actual: &str) -> bool {
+    // Normalize for Windows paths.
+    let actual = actual.replace('\\', "/");
+    actual.lines().any(|mut line| {
+        for (i, part) in expected.split("[..]").enumerate() {
+            match line.find(part) {
+                Some(j) => {
+                    if i == 0 && j != 0 {
+                        return false;
+                    }
+                    line = &line[j + part.len()..];
+                }
+                None => return false,
+            }
+        }
+        line.is_empty() || expected.ends_with("[..]")
+    })
+}
+
 fn valid_test(dirname: &str) {
     let (status, stdout, stderr) = run(dirname);
     if !status.success() {
@@ -48,30 +67,47 @@ fn valid() {
 
 #[test]
 fn basic_broken() {
-    broken_test("basic_broken", "bar.html");
+    broken_test("basic_broken", "foo.html:3: broken link - `bar.html`");
 }
 
 #[test]
 fn broken_fragment_local() {
-    broken_test("broken_fragment_local", "#somefrag");
+    broken_test(
+        "broken_fragment_local",
+        "foo.html:3: broken link fragment `#somefrag` pointing to `foo.html`",
+    );
 }
 
 #[test]
 fn broken_fragment_remote() {
-    broken_test("broken_fragment_remote/inner", "#somefrag");
+    broken_test(
+        "broken_fragment_remote/inner",
+        "foo.html:3: broken link fragment `#somefrag` pointing to \
+         `[..]/broken_fragment_remote/bar.html`",
+    );
 }
 
 #[test]
 fn broken_redir() {
-    broken_test("broken_redir", "sometarget");
+    broken_test(
+        "broken_redir",
+        "foo.html:3: broken redirect from `redir-bad.html` to `sometarget`",
+    );
 }
 
 #[test]
 fn directory_link() {
-    broken_test("directory_link", "somedir");
+    broken_test(
+        "directory_link",
+        "foo.html:3: directory link to `somedir` (directory links should use index.html instead)",
+    );
 }
 
 #[test]
 fn redirect_loop() {
-    broken_test("redirect_loop", "redir-bad.html");
+    broken_test(
+        "redirect_loop",
+        "foo.html:3: redirect from `redir-bad.html` to `[..]redirect_loop/redir-bad.html` \
+         which is also a redirect (not supported)",
+    );
 }
diff --git a/src/tools/linkchecker/tests/redirect_loop/redir-bad.html b/src/tools/linkchecker/tests/redirect_loop/redir-bad.html
@@ -2,6 +2,7 @@
 <html lang="en">
 <head>
     <meta http-equiv="refresh" content="0;URL=redir-bad.html">
+    <title>Redirection</title>
 </head>
 <body>
     <p>Redirecting to <a href="redir-bad.html">redir-bad.html</a>...</p>
diff --git a/src/tools/linkchecker/tests/valid/inner/redir-bad.html b/src/tools/linkchecker/tests/valid/inner/redir-bad.html
@@ -2,6 +2,7 @@
 <html lang="en">
 <head>
     <meta http-equiv="refresh" content="0;URL=xxx">
+    <title>Redirection</title>
 </head>
 <body>
     <p>Redirecting to <a href="xxx">xxx</a>...</p>
diff --git a/src/tools/linkchecker/tests/valid/inner/redir.html b/src/tools/linkchecker/tests/valid/inner/redir.html
@@ -2,6 +2,7 @@
 <html lang="en">
 <head>
     <meta http-equiv="refresh" content="0;URL=redir-target.html">
+    <title>Redirection</title>
 </head>
 <body>
     <p>Redirecting to <a href="redir-target.html">redir-target.html</a>...</p>

Original file line number	Diff line number	Diff line change
`@@ -347,7 +347,7 @@ impl Checker {`
`347`	`347`	`} else {`
`348`	`348`	`report.errors += 1;`
`349`	`349`	`print!("{}:{}: broken link fragment ", pretty_path, i + 1);`
`350`		- println!("`#{}` pointing to `{}`", fragment, pretty_path);
	`350`	+ println!("`#{}` pointing to `{}`", fragment, target_pretty_path);
`351`	`351`	`};`
`352`	`352`	`}`
`353`	`353`	`});`