Rollup merge of #94848 - GuillaumeGomez:browser-ui-test-version, r=Mark-Simulacrum

Compare installed browser-ui-test version to the one used in CI

I happened a few times to run into (local) rustdoc GUI tests errors because I forgot to update my browser-ui-test version. I know at least two others who encountered the same problem so I think emitting a warning to let us know about this version mismatch would make it easier to figure out.

So now, I'm not too sure that this PR is the right approach because it requires to parse a Dockerfile, which feels pretty bad. I had the idea to instead store the browser-ui-test version into a docker ARG like:

```docker
ARG BROWSER_UI_TEST_VERSION=0.8.0
```

And then use it as such in the command to make the parsing more reliable.

Or we could store this version into a file and import this file into the Dockerfile and read it from the builder.

Any preference or maybe another solution?

r? ``@Mark-Simulacrum``

src/bootstrap/test.rs

@@ -836,9 +836,9 @@ impl Step for RustdocJSNotStd {
     }
 }
 
-fn check_if_browser_ui_test_is_installed_global(npm: &Path, global: bool) -> bool {
+fn get_browser_ui_test_version_inner(npm: &Path, global: bool) -> Option<String> {
     let mut command = Command::new(&npm);
-    command.arg("list").arg("--depth=0");
+    command.arg("list").arg("--parseable").arg("--long").arg("--depth=0");
     if global {
         command.arg("--global");
     }
@@ -846,12 +846,29 @@ fn check_if_browser_ui_test_is_installed_global(npm: &Path, global: bool) -> boo
         .output()
         .map(|output| String::from_utf8_lossy(&output.stdout).into_owned())
         .unwrap_or(String::new());
-    lines.contains(&" browser-ui-test@")
+    lines.lines().find_map(|l| l.split(":browser-ui-test@").skip(1).next()).map(|v| v.to_owned())
 }
 
-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)
+fn get_browser_ui_test_version(npm: &Path) -> Option<String> {
+    get_browser_ui_test_version_inner(npm, false)
+        .or_else(|| get_browser_ui_test_version_inner(npm, true))
+}
+
+fn compare_browser_ui_test_version(installed_version: &str, src: &Path) {
+    match fs::read_to_string(
+        src.join("src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version"),
+    ) {
+        Ok(v) => {
+            if v.trim() != installed_version {
+                eprintln!(
+                    "⚠️ Installed version of browser-ui-test (`{}`) is different than the \
+                     one used in the CI (`{}`)",
+                    installed_version, v
+                );
+            }
+        }
+        Err(e) => eprintln!("Couldn't find the CI browser-ui-test version: {:?}", e),
+    }
 }
 
 #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
@@ -874,7 +891,7 @@ impl Step for RustdocGUI {
                     .config
                     .npm
                     .as_ref()
-                    .map(|p| check_if_browser_ui_test_is_installed(p))
+                    .map(|p| get_browser_ui_test_version(p).is_some())
                     .unwrap_or(false)
         }))
     }
@@ -892,7 +909,13 @@ impl Step for RustdocGUI {
 
         // 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) {
+        match get_browser_ui_test_version(&npm) {
+            Some(version) => {
+                // We also check the version currently used in CI and emit a warning if it's not the
+                // same one.
+                compare_browser_ui_test_version(&version, &builder.build.src);
+            }
+            None => {
                 eprintln!(
                     "error: rustdoc-gui test suite cannot be run because npm `browser-ui-test` \
                      dependency is missing",
@@ -903,6 +926,7 @@ impl Step for RustdocGUI {
                 );
                 panic!("Cannot run rustdoc-gui tests");
             }
+        }
 
         let out_dir = builder.test_out(self.target).join("rustdoc-gui");
 

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

@@ -65,14 +65,20 @@ 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}"
+ENV NODE_FOLDER=/node-v14.4.0-linux-x64/bin
+ENV PATH="$NODE_FOLDER:${PATH}"
+
+COPY host-x86_64/x86_64-gnu-tools/browser-ui-test.version /tmp/
 
 # 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.8.3 --unsafe-perm=true
+#
+# The `browser-ui-test.version` file is also used by bootstrap to emit warnings in case
+# the local version of the package is different than the one used by the CI.
+RUN npm install -g browser-ui-test@$(head -n 1 /tmp/browser-ui-test.version) --unsafe-perm=true
 
 ENV RUST_CONFIGURE_ARGS \
   --build=x86_64-unknown-linux-gnu \

src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version

@@ -0,0 +1 @@
+0.8.3

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

@@ -26,6 +26,7 @@ if [[ -n "${CI_ONLY_WHEN_SUBMODULES_CHANGED-}" ]]; then
              src/test/rustdoc-gui \
              src/librustdoc \
              src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile \
+             src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version \
              src/tools/rustdoc-gui); then
         # There was a change in either rustdoc or in its GUI tests.
         echo "Rustdoc was updated"