Auto merge of rust-lang#84586 - GuillaumeGomez:enforce-rustdoc-gui-te…

…st-suite-run, r=Mark-Simulacrum

Enforce rustdoc-gui test-suite run

Part of rust-lang#84550

src/bootstrap/mk/Makefile.in

@@ -45,10 +45,6 @@ check-aux:
 		src/tools/cargo \
 		src/tools/cargotest \
 		$(BOOTSTRAP_ARGS)
-check-aux-and-gui: check-aux
-	$(Q)$(BOOTSTRAP) test --stage 2 \
-		src/test/rustdoc-gui \
-		$(BOOTSTRAP_ARGS)
 check-bootstrap:
 	$(Q)$(CFG_PYTHON) $(CFG_SRC_DIR)src/bootstrap/bootstrap_test.py
 dist:

src/bootstrap/test.rs

@@ -774,6 +774,24 @@ impl Step for RustdocJSNotStd {
     }
 }
 
+fn check_if_browser_ui_test_is_installed_global(npm: &Path, global: bool) -> bool {
+    let mut command = Command::new(&npm);
+    command.arg("list").arg("--depth=0");
+    if global {
+        command.arg("--global");
+    }
+    let lines = command
+        .output()
+        .map(|output| String::from_utf8_lossy(&output.stdout).into_owned())
+        .unwrap_or(String::new());
+    lines.contains(&" browser-ui-test@")
+}
+
+fn check_if_browser_ui_test_is_installed(npm: &Path) -> bool {
+    check_if_browser_ui_test_is_installed_global(npm, false)
+        || check_if_browser_ui_test_is_installed_global(npm, true)
+}
+
 #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
 pub struct RustdocGUI {
     pub target: TargetSelection,
@@ -786,7 +804,17 @@ impl Step for RustdocGUI {
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
-        run.path("src/test/rustdoc-gui")
+        let builder = run.builder;
+        let run = run.path("src/test/rustdoc-gui");
+        run.default_condition(
+            builder.config.nodejs.is_some()
+                && builder
+                    .config
+                    .npm
+                    .as_ref()
+                    .map(|p| check_if_browser_ui_test_is_installed(p))
+                    .unwrap_or(false),
+        )
     }
 
     fn make_run(run: RunConfig<'_>) {
@@ -795,58 +823,54 @@ impl Step for RustdocGUI {
     }
 
     fn run(self, builder: &Builder<'_>) {
-        if let (Some(nodejs), Some(npm)) = (&builder.config.nodejs, &builder.config.npm) {
-            builder.ensure(compile::Std { compiler: self.compiler, target: self.target });
-
-            // The goal here is to check if the necessary packages are installed, and if not, we
-            // display a warning and move on.
-            let mut command = Command::new(&npm);
-            command.arg("list").arg("--depth=0");
-            let lines = command
-                .output()
-                .map(|output| String::from_utf8_lossy(&output.stdout).to_string())
-                .unwrap_or(String::new());
-            if !lines.contains(&" browser-ui-test@") {
-                println!(
-                    "warning: rustdoc-gui test suite cannot be run because npm `browser-ui-test` \
-                     dependency is missing",
-                );
-                println!(
-                    "If you want to install the `{0}` dependency, run `npm install {0}`",
-                    "browser-ui-test",
-                );
-                return;
-            }
+        let nodejs = builder.config.nodejs.as_ref().expect("nodejs isn't available");
+        let npm = builder.config.npm.as_ref().expect("npm isn't available");
 
-            let out_dir = builder.test_out(self.target).join("rustdoc-gui");
+        builder.ensure(compile::Std { compiler: self.compiler, target: self.target });
 
-            // We remove existing folder to be sure there won't be artifacts remaining.
-            let _ = fs::remove_dir_all(&out_dir);
+        // The goal here is to check if the necessary packages are installed, and if not, we
+        // panic.
+        if !check_if_browser_ui_test_is_installed(&npm) {
+            eprintln!(
+                "error: rustdoc-gui test suite cannot be run because npm `browser-ui-test` \
+                 dependency is missing",
+            );
+            eprintln!(
+                "If you want to install the `{0}` dependency, run `npm install {0}`",
+                "browser-ui-test",
+            );
+            panic!("Cannot run rustdoc-gui tests");
+        }
 
-            // We generate docs for the libraries present in the rustdoc-gui's src folder.
-            let libs_dir = builder.build.src.join("src/test/rustdoc-gui/src");
-            for entry in libs_dir.read_dir().expect("read_dir call failed") {
-                let entry = entry.expect("invalid entry");
-                let path = entry.path();
-                if path.extension().map(|e| e == "rs").unwrap_or(false) {
-                    let mut command = builder.rustdoc_cmd(self.compiler);
-                    command.arg(path).arg("-o").arg(&out_dir);
-                    builder.run(&mut command);
-                }
-            }
+        let out_dir = builder.test_out(self.target).join("rustdoc-gui");
 
-            // We now run GUI tests.
-            let mut command = Command::new(&nodejs);
-            command
-                .arg(builder.build.src.join("src/tools/rustdoc-gui/tester.js"))
-                .arg("--doc-folder")
-                .arg(out_dir)
-                .arg("--tests-folder")
-                .arg(builder.build.src.join("src/test/rustdoc-gui"));
-            builder.run(&mut command);
-        } else {
-            builder.info("No nodejs found, skipping \"src/test/rustdoc-gui\" tests");
+        // We remove existing folder to be sure there won't be artifacts remaining.
+        let _ = fs::remove_dir_all(&out_dir);
+
+        let mut nb_generated = 0;
+        // We generate docs for the libraries present in the rustdoc-gui's src folder.
+        let libs_dir = builder.build.src.join("src/test/rustdoc-gui/src");
+        for entry in libs_dir.read_dir().expect("read_dir call failed") {
+            let entry = entry.expect("invalid entry");
+            let path = entry.path();
+            if path.extension().map(|e| e == "rs").unwrap_or(false) {
+                let mut command = builder.rustdoc_cmd(self.compiler);
+                command.arg(path).arg("-o").arg(&out_dir);
+                builder.run(&mut command);
+                nb_generated += 1;
+            }
         }
+        assert!(nb_generated > 0, "no documentation was generated...");
+
+        // We now run GUI tests.
+        let mut command = Command::new(&nodejs);
+        command
+            .arg(builder.build.src.join("src/tools/rustdoc-gui/tester.js"))
+            .arg("--doc-folder")
+            .arg(out_dir)
+            .arg("--tests-folder")
+            .arg(builder.build.src.join("src/test/rustdoc-gui"));
+        builder.run(&mut command);
     }
 }
 

src/ci/docker/host-x86_64/x86_64-gnu-aux/Dockerfile

@@ -17,27 +17,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
   libgl1-mesa-dev \
   llvm-dev \
   libfreetype6-dev \
-  libexpat1-dev \
-  libexpat1-dev \
-  gnupg \
-  apt-utils \
-  wget \
-  fonts-ipafont-gothic \
-  fonts-wqy-zenhei \
-  fonts-thai-tlwg \
-  fonts-kacst \
-  fonts-freefont-ttf \
-  libxss1 \
-  libxtst6
-
-RUN curl -sL https://nodejs.org/dist/v14.4.0/node-v14.4.0-linux-x64.tar.xz | tar -xJ
-ENV PATH="/node-v14.4.0-linux-x64/bin:${PATH}"
-
-# Install required dependencies from browser-UI-test framework
-# For now, we need to use `--unsafe-perm=true` to go around an issue when npm tries
-# to create a new folder. For reference:
-# https://github.com/puppeteer/puppeteer/issues/375
-RUN npm install browser-ui-test -g --unsafe-perm=true
+  libexpat1-dev
 
 COPY scripts/sccache.sh /scripts/
 RUN sh /scripts/sccache.sh
@@ -46,4 +26,4 @@ COPY scripts/cmake.sh /scripts/
 RUN /scripts/cmake.sh
 
 ENV RUST_CONFIGURE_ARGS --build=x86_64-unknown-linux-gnu
-ENV RUST_CHECK_TARGET check-aux-and-gui
+ENV RUST_CHECK_TARGET check-aux

src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile

@@ -12,8 +12,48 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
   cmake \
   libssl-dev \
   sudo \
-  xz-utils \
-  pkg-config
+  xz-utils
+
+# Install dependencies for chromium browser
+RUN apt-get install -y \
+  gconf-service \
+  libasound2 \
+  libatk1.0-0 \
+  libatk-bridge2.0-0 \
+  libc6 \
+  libcairo2 \
+  libcups2 \
+  libdbus-1-3 \
+  libexpat1 \
+  libfontconfig1 \
+  libgcc1 \
+  libgconf-2-4 \
+  libgdk-pixbuf2.0-0 \
+  libglib2.0-0 \
+  libgtk-3-0 \
+  libnspr4 \
+  libpango-1.0-0 \
+  libpangocairo-1.0-0 \
+  libstdc++6 \
+  libx11-6 \
+  libx11-xcb1 \
+  libxcb1 \
+  libxcomposite1 \
+  libxcursor1 \
+  libxdamage1 \
+  libxext6 \
+  libxfixes3 \
+  libxi6 \
+  libxrandr2 \
+  libxrender1 \
+  libxss1 \
+  libxtst6 \
+  fonts-liberation \
+  libappindicator1 \
+  libnss3 \
+  lsb-release \
+  xdg-utils \
+  wget
 
 COPY scripts/sccache.sh /scripts/
 RUN sh /scripts/sccache.sh
@@ -23,7 +63,19 @@ RUN /scripts/cmake.sh
 
 COPY host-x86_64/x86_64-gnu-tools/checktools.sh /tmp/
 
+RUN curl -sL https://nodejs.org/dist/v14.4.0/node-v14.4.0-linux-x64.tar.xz | tar -xJ
+ENV PATH="/node-v14.4.0-linux-x64/bin:${PATH}"
+
+# For now, we need to use `--unsafe-perm=true` to go around an issue when npm tries
+# to create a new folder. For reference:
+# https://github.com/puppeteer/puppeteer/issues/375
+#
+# We also specify the version in case we need to update it to go around cache limitations.
+RUN npm install -g browser-ui-test@0.2.12 --unsafe-perm=true
+
 ENV RUST_CONFIGURE_ARGS \
   --build=x86_64-unknown-linux-gnu \
   --save-toolstates=/tmp/toolstate/toolstates.json
-ENV SCRIPT /tmp/checktools.sh ../x.py
+
+ENV SCRIPT /tmp/checktools.sh ../x.py && \
+  NODE_PATH=`npm root -g` python3 ../x.py test src/test/rustdoc-gui --stage 2

src/ci/scripts/should-skip-this.sh

@@ -26,6 +26,12 @@ elif ! git diff --quiet "$BASE_COMMIT" -- src/tools/clippy src/tools/rustfmt; th
     # There is not an easy blanket search for subtrees. For now, manually list
     # the subtrees.
     echo "Executing the job since clippy or rustfmt subtree was updated"
+elif ! (git diff --quiet "$BASE_COMMIT" -- \
+         src/test/rustdoc-gui \
+         src/librustdoc \
+         src/tools/rustdoc-gui); then
+    # There was a change in either rustdoc or in its GUI tests.
+    echo "Executing the job since rustdoc was updated"
 else
     echo "Not executing this job since no submodules nor subtrees were updated"
     ciCommandSetEnv SKIP_JOB 1