tests: Migrate create_slide.list.sh into cargo xtask function. (#2957)

The new xtask function makes the helper code
- more readable
- more reliable due to better error checking
- be in the same place as other helper functions
- and more aligned to the skillset relevant for contributing in this
repository.

The shell script grew and was not readable for everyone anymore without
deeper knowledge.

mitigates #2941 in a more reliable way but does still not fully fix the
root cause

.github/workflows/build.yml

@@ -162,12 +162,7 @@ jobs:
         working-directory: ./tests
       - name: Test Javascript
         if: matrix.language == 'en'
-        run: |
+        run: cargo xtask web-tests --dir book/comprehensive-rust-${{ matrix.language }}/html
-          ./src/slides/create-slide.list.sh
-          npm test
-        env:
-          TEST_BOOK_DIR: ../book/comprehensive-rust-${{ matrix.language }}/html
-        working-directory: ./tests
 
   po-diff:
     name: Translation diff

Cargo.lock

@@ -3554,6 +3554,7 @@ version = "0.1.0"
 dependencies = [
  "anyhow",
  "clap",
+ "walkdir",
 ]
 
 [[package]]

tests/src/slides/create-slide.list.sh

@@ -1,80 +0,0 @@
-#!/bin/bash
-
-# This script (re)creates the slides.list.ts file based on the given book html directory.
-# It is used to regenerate the list of slides that are tested in the slide-size.test.ts file.
-# Takes either TEST_BOOK_DIR environment variable or first parameter as override.
-
-set -e
-BASEDIR="$(dirname "$0")"
-
-if [[ -n "$1" ]]; then
-    # take directory from command line
-    TEST_BOOK_DIR="$1"
-fi
-
-# check if TEST_BOOK_DIR is empty (not set by environment nor parameter)
-if [[ -z "${TEST_BOOK_DIR}" ]]; then
-  echo "Usage: $0 <book_html_dir>"
-  exit 1
-fi
-
-# check if this is the correct root directory by checking if it contains the index.html
-if [[ ! -f "${TEST_BOOK_DIR}/index.html" ]]; then
-  echo "Could not find index.html in ${TEST_BOOK_DIR}. Please check if the correct directory is used (e.g. book/html). You might need to (re)create the directory with mdbook build."
-  exit 1
-fi
-
-# These special pages should never be tested for slide size.
-EXCLUDE_FILES=(
-  "exercise.html"
-  "solution.html"
-  "toc.html"
-  "print.html"
-  "404.html"
-  "glossary.html"
-  "index.html"
-  "course-structure.html"
-)
-
-CANDIDATE_SLIDES=""
-if [[ -n "${CI}" ]]; then
-  echo "CI environment detected, checking only changed slides."
-  # Find changed markdown files in src/ and map them to their html output.
-  # GITHUB_BASE_REF is available in PRs. Default to 'main' for other CI contexts.
-  CANDIDATE_SLIDES=$(git diff --name-only "origin/${GITHUB_BASE_REF:-main}"... \
-    | grep '^src/.*\.md$' \
-    | sed 's|^src/||; s|\.md$|.html|' \
-    || true)
-else
-  # TODO: Limit the amount of files to check: Figure out what a good local diff base is.
-  echo "Local environment, checking all slides."
-  # Find all .html files recursively.
-  CANDIDATE_SLIDES=$(find "${TEST_BOOK_DIR}" -name "*.html" -printf "%P\n")
-fi
-
-SLIDES=""
-if [[ -n "${CANDIDATE_SLIDES}" ]]; then
-  # From the candidate slides, filter out:
-  # - Files that are just redirects.
-  # - Files that are in the EXCLUDE_FILES list.
-  # - Files that do not exist in the TEST_BOOK_DIR (by using ls)
-  pushd "${TEST_BOOK_DIR}" > /dev/null
-  EXCLUDE_PATTERN=$(IFS="|" ; echo "${EXCLUDE_FILES[*]}")
-  SLIDES=$(echo "${CANDIDATE_SLIDES}" | grep -v -E "${EXCLUDE_PATTERN}" \
-    | xargs ls 2>/dev/null \
-    | xargs -r grep -L "Redirecting to...") || true
-  popd > /dev/null
-fi
-
-if [[ -n "${CI}" ]]; then
-  echo "The following slides will be checked:"
-  echo "${SLIDES}"
-fi
-OUTPUT="${BASEDIR}/slides.list.ts"
-
-# create a ts module that can be imported in the tests
-echo "export const slides = [" > ${OUTPUT};
-for SLIDE in ${SLIDES}; do
-echo "  \"${SLIDE}\"," >> ${OUTPUT};
-done;
-echo "];" >> ${OUTPUT};

tests/src/slides/slides.list.ts

@@ -1,5 +1,5 @@
 // to enable local testing for slide size checks please (re)generate this file by executing:
-// $ ./tests/src/slides/create-slide.list.sh book/html
+// $ cargo xtask create-slide-list --dir book/html/
 //
 // This file is on purpose not pre-filled in the repository to avoid
 // a) manual maintenance of slide list

xtask/Cargo.toml

@@ -7,3 +7,4 @@ publish = false
 [dependencies]
 anyhow = "1.0.100"
 clap = { version = "4.5.48", features = ["derive"] }
+walkdir = "2.5.0"

xtask/src/main.rs

@@ -21,9 +21,10 @@
 
 use anyhow::{Context, Result, anyhow};
 use clap::{Parser, Subcommand};
-use std::env;
 use std::path::{Path, PathBuf};
 use std::process::Command;
+use std::{env, fs};
+use walkdir::WalkDir;
 
 fn main() -> Result<()> {
     execute_task()
@@ -54,6 +55,13 @@ enum Task {
         #[arg(short, long)]
         dir: Option<PathBuf>,
     },
+    /// (Re)creates the slides.list.ts file based on the given book html
+    /// directory.
+    CreateSlideList {
+        /// The book html directory
+        #[arg(short, long)]
+        dir: PathBuf,
+    },
     /// Tests all included Rust snippets.
     RustTests,
     /// Starts a web server with the course.
@@ -85,6 +93,7 @@ fn execute_task() -> Result<()> {
     match cli.task {
         Task::InstallTools { binstall } => install_tools(binstall),
         Task::WebTests { dir } => run_web_tests(dir),
+        Task::CreateSlideList { dir } => create_slide_list(dir),
         Task::RustTests => run_rust_tests(),
         Task::Serve { language, output } => start_web_server(language, output),
         Task::Build { language, output } => build(language, output),
@@ -191,13 +200,7 @@ fn run_web_tests(dir: Option<PathBuf>) -> Result<()> {
 
     if let Some(d) = &absolute_dir {
         println!("Refreshing slide lists...");
-        let refresh_slides_script = Path::new("tests")
+        create_slide_list(d.clone())?;
-            .join("src")
-            .join("slides")
-            .join("create-slide.list.sh");
-        let mut cmd = Command::new(&refresh_slides_script);
-        cmd.current_dir(workspace_dir).arg(d);
-        run_command(&mut cmd)?;
     }
 
     let tests_dir = workspace_dir.join("tests");
@@ -210,6 +213,125 @@ fn run_web_tests(dir: Option<PathBuf>) -> Result<()> {
     run_command(&mut cmd)
 }
 
+// Creates a list of .html slides from the html directory containing the
+// index.html to check the slides.
+// - CI environment: Only modified files are listed
+// - Otherwise: All existing html files
+fn create_slide_list(html_directory: PathBuf) -> Result<()> {
+    let workspace_dir = Path::new(env!("CARGO_WORKSPACE_DIR"));
+    let tests_dir = workspace_dir.join("tests");
+
+    // Check if the provided directory is correct
+    if !html_directory.join("index.html").exists() {
+        return Err(anyhow!(
+            "Could not find index.html in {}. Please check if the correct directory is used (e.g. book/html).",
+            html_directory.display()
+        ));
+    }
+
+    // These special slides are not checked against the style guide
+    let exclude_paths = [
+        "exercise.html",
+        "solution.html",
+        "toc.html",
+        "print.html",
+        "404.html",
+        "glossary.html",
+        "index.html",
+        "course-structure.html",
+    ]
+    .map(PathBuf::from);
+
+    // Collect the files relevant for evaluation.
+    // - CI environment variable is set: all modified markdown files in the src/
+    //   directory
+    // - all html files in the provided directory otherwise
+    let candidate_slides: Vec<PathBuf> = if env::var("CI").is_ok() {
+        println!("CI environment detected, checking only modified slides.");
+        // GITHUB_BASE_REF is available in PRs. Default to 'main' for other CI
+        // contexts.
+        let base_ref = env::var("GITHUB_BASE_REF").unwrap_or("main".to_string());
+        let mut cmd = Command::new("git");
+        cmd.arg("diff")
+            .arg("--name-only")
+            .arg(format!("{}...", base_ref))
+            .arg("--")
+            // Retrieve all modified files in the src directory.
+            // Pathspec syntax: https://git-scm.com/docs/gitglossary#Documentation/gitglossary.txt-pathspec
+            // `*` can match path separators, thus matches also files in
+            // subdirectories
+            .arg("src/*.md");
+        println!("> {cmd:?}");
+        let output = cmd.output().context("Failed to run git diff")?;
+        String::from_utf8(output.stdout)?
+            .lines()
+            .map(|line| {
+                let path = Path::new(line);
+                // We know the path starts with "src/" because of the pathspec in the
+                // `git diff` command, and we need it relative to the html base
+                // directory
+                let stripped_path = path.strip_prefix("src").unwrap();
+                let mut html_path = stripped_path.to_path_buf();
+                // replace the .md extension with .html
+                html_path.set_extension("html");
+                html_path
+            })
+            .collect()
+    } else {
+        println!("Local environment, checking all slides.");
+        WalkDir::new(&html_directory)
+            .into_iter()
+            .filter_map(|e| e.ok())
+            // only files with .html extension
+            .filter(|e| e.path().extension().is_some_and(|ext| ext == "html"))
+            // relative path inside the html directory
+            .map(|e| e.path().strip_prefix(&html_directory).unwrap().to_path_buf())
+            .collect()
+    };
+    // Filter the candidate slides
+    let mut slides = Vec::new();
+    for slide in candidate_slides {
+        // Skip excluded files
+        if exclude_paths.iter().any(|exclude_path| slide.ends_with(exclude_path)) {
+            continue;
+        }
+
+        // Test if the html files actually exist
+        let full_path = html_directory.join(&slide);
+        if !full_path.exists() {
+            continue;
+        }
+
+        // Optimization: check if these are redirection html files and skip these
+        let content = fs::read_to_string(&full_path)
+            .with_context(|| format!("Failed to read slide: {}", slide.display()))?;
+        if content.contains("Redirecting to...") {
+            continue;
+        }
+        slides.push(slide);
+    }
+
+    if env::var("CI").is_ok() {
+        println!("The following slides have been modified and will be checked:");
+        for slide in &slides {
+            println!("{}", slide.display());
+        }
+    }
+
+    // Write the file list into a .ts file that can be read by the JS based webtest
+    let output_path = tests_dir.join("src").join("slides").join("slides.list.ts");
+    let mut output_content = "export const slides = [\n".to_string();
+    for slide in slides {
+        output_content.push_str(&format!("  \"{}\",\n", slide.display()));
+    }
+    output_content.push_str("];\n");
+
+    fs::write(&output_path, output_content)
+        .with_context(|| format!("Failed to write to {}", output_path.display()))?;
+
+    Ok(())
+}
+
 fn run_rust_tests() -> Result<()> {
     println!("Running rust tests...");
     let workspace_root = Path::new(env!("CARGO_WORKSPACE_DIR"));