Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,7 @@ jobs:
working-directory: ./tests
- name: Test Javascript
if: matrix.language == 'en'
run: |
./src/slides/create-slide.list.sh
npm test
env:
TEST_BOOK_DIR: ../book/comprehensive-rust-${{ matrix.language }}/html
working-directory: ./tests
run: cargo xtask web-tests --dir book/comprehensive-rust-${{ matrix.language }}/html

po-diff:
name: Translation diff
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

80 changes: 0 additions & 80 deletions tests/src/slides/create-slide.list.sh

This file was deleted.

2 changes: 1 addition & 1 deletion tests/src/slides/slides.list.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
1 change: 1 addition & 0 deletions xtask/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ publish = false
[dependencies]
anyhow = "1.0.100"
clap = { version = "4.5.48", features = ["derive"] }
walkdir = "2.5.0"
138 changes: 130 additions & 8 deletions xtask/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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")
.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)?;
create_slide_list(d.clone())?;
}

let tests_dir = workspace_dir.join("tests");
Expand All @@ -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))
Copy link
Collaborator Author

@michael-kerscher michael-kerscher Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is one of the things I'm not 100% sure if this would create issues. This git diff should provide the list of filenames (--name-only) that was changed by this PR (over all commits). But I'm not sure about GITHUB_BASE_REF in various situations. I might need to use a more reliable way to do this

Copy link
Collaborator Author

@michael-kerscher michael-kerscher Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GITHUB_BASE_REF might only be set in forked repositories (but not sure if this is the case), which would explain why I did not see issues with the previous shell script

.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"));
Expand Down
Loading