From 0dadc526250a04d77243e1e1810b181c8bfd1b38 Mon Sep 17 00:00:00 2001 From: Ryan Orendorff <12442942+ryanorendorff@users.noreply.github.com> Date: Fri, 17 Oct 2025 10:28:05 -0600 Subject: [PATCH 1/6] move test fixtures to cargo test Move test fixtures from manual mdbook builds to proper cargo tests. Add test-util feature flag to gate CheckCodePreprocessor test utilities so that the e2e tests can run without approval checking. All tests can be run with `cargo test --features test-util`. --- Cargo.toml | 13 +- README.md | 17 +- src/approval.rs | 3 + src/extractor.rs | 7 +- src/language.rs | 10 +- src/lib.rs | 29 +++ src/preprocessor.rs | 52 +++++- tests/common/mod.rs | 100 +++++++++++ tests/fixtures/error_cases/book.toml | 17 ++ tests/fixtures/error_cases/src/SUMMARY.md | 3 + tests/fixtures/error_cases/src/test_error.md | 12 ++ tests/fixtures/{ => valid_cases}/book.toml | 0 .../fixtures/{ => valid_cases}/src/SUMMARY.md | 0 tests/fixtures/{ => valid_cases}/src/intro.md | 0 .../src/other_langs/README.md | 0 .../src/other_langs/c_examples/README.md | 0 .../src/other_langs/c_examples/c_example.md | 0 .../src/other_langs/ts_examples/README.md | 0 .../ts_examples/typescript_example.md | 0 .../src/parasol_examples/README.md | 0 .../src/parasol_examples/basic.md | 0 .../src/parasol_examples/propagate.md | 0 .../{ => valid_cases}/src/solidity_example.md | 0 .../{ => valid_cases}/src/test_error.md | 0 tests/integration.rs | 165 ++++++++++++++++++ 25 files changed, 407 insertions(+), 21 deletions(-) create mode 100644 src/lib.rs create mode 100644 tests/common/mod.rs create mode 100644 tests/fixtures/error_cases/book.toml create mode 100644 tests/fixtures/error_cases/src/SUMMARY.md create mode 100644 tests/fixtures/error_cases/src/test_error.md rename tests/fixtures/{ => valid_cases}/book.toml (100%) rename tests/fixtures/{ => valid_cases}/src/SUMMARY.md (100%) rename tests/fixtures/{ => valid_cases}/src/intro.md (100%) rename tests/fixtures/{ => valid_cases}/src/other_langs/README.md (100%) rename tests/fixtures/{ => valid_cases}/src/other_langs/c_examples/README.md (100%) rename tests/fixtures/{ => valid_cases}/src/other_langs/c_examples/c_example.md (100%) rename tests/fixtures/{ => valid_cases}/src/other_langs/ts_examples/README.md (100%) rename tests/fixtures/{ => valid_cases}/src/other_langs/ts_examples/typescript_example.md (100%) rename tests/fixtures/{ => valid_cases}/src/parasol_examples/README.md (100%) rename tests/fixtures/{ => valid_cases}/src/parasol_examples/basic.md (100%) rename tests/fixtures/{ => valid_cases}/src/parasol_examples/propagate.md (100%) rename tests/fixtures/{ => valid_cases}/src/solidity_example.md (100%) rename tests/fixtures/{ => valid_cases}/src/test_error.md (100%) create mode 100644 tests/integration.rs diff --git a/Cargo.toml b/Cargo.toml index 7ba4b01..27c9abc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,10 @@ version = "0.1.0" edition = "2021" license = "AGPL-3.0-only" +[lib] +name = "mdbook_check_code" +path = "src/lib.rs" + [[bin]] name = "mdbook-check-code" path = "src/main.rs" @@ -22,6 +26,13 @@ chrono = "0.4" clap = { version = "4.5", features = ["derive", "cargo"] } sha2 = "0.10.9" directories = "6.0.0" -tokio = { version = "1.42", features = ["process", "fs", "io-util"] } +tokio = { version = "1.42", features = ["process", "fs", "io-util", "rt"] } futures = "0.3" num_cpus = "1.16" + +[dev-dependencies] +tokio = { version = "1.42", features = ["macros", "rt-multi-thread"] } + +[features] +default = [] +test-util = [] diff --git a/README.md b/README.md index d65c5a3..272b7e8 100644 --- a/README.md +++ b/README.md @@ -90,15 +90,20 @@ Optional: ## Testing -Test with included fixtures: +Run the test suite (requires compilers: gcc, clang with parasol target, tsc, solc): ```bash -# With all compilers (Nix) -nix develop --command bash -c "cd tests/fixtures && mdbook build" +# Run all tests (unit + integration) +cargo test --features test-util -# Or with Cargo -cargo build --release -cd tests/fixtures && mdbook build +# Run only unit tests (no compilers required) +cargo test --lib + +# Run integration tests +cargo test --test integration --features test-util + +# With Nix (provides all compilers) +nix develop --command cargo test --features test-util ``` ## License diff --git a/src/approval.rs b/src/approval.rs index 8ec9e17..37e0994 100644 --- a/src/approval.rs +++ b/src/approval.rs @@ -39,6 +39,7 @@ pub fn is_approved(book_toml_path: &Path) -> Result { } /// Approve a book.toml +#[allow(dead_code)] // Used by CLI binary (main.rs), not library pub fn approve(book_toml_path: &Path) -> Result<()> { let content = fs::read_to_string(book_toml_path) .with_context(|| format!("Failed to read {}", book_toml_path.display()))?; @@ -65,6 +66,7 @@ pub fn approve(book_toml_path: &Path) -> Result<()> { } /// Deny (remove approval) for a book.toml +#[allow(dead_code)] // Used by CLI binary (main.rs), not library pub fn deny(book_toml_path: &Path) -> Result<()> { let content = fs::read_to_string(book_toml_path) .with_context(|| format!("Failed to read {}", book_toml_path.display()))?; @@ -85,6 +87,7 @@ pub fn deny(book_toml_path: &Path) -> Result<()> { } /// List all approved books +#[allow(dead_code)] // Used by CLI binary (main.rs), not library pub fn list_approved() -> Result> { let approval_dir = get_approval_dir()?; diff --git a/src/extractor.rs b/src/extractor.rs index 19bfbed..3120e64 100644 --- a/src/extractor.rs +++ b/src/extractor.rs @@ -60,9 +60,8 @@ pub struct CodeBlock { /// /// # Example /// -/// ```ignore -/// let markdown = r#" -/// # My Code +/// ````ignore +/// let markdown = r#"# My Code /// /// ```c /// int main() { return 0; } @@ -72,7 +71,7 @@ pub struct CodeBlock { /// let blocks = extract_code_blocks(markdown); /// assert_eq!(blocks.len(), 1); /// assert_eq!(blocks[0].language, "c"); -/// ``` +/// ```` pub fn extract_code_blocks(content: &str) -> Vec { let parser = Parser::new(content); let mut code_blocks = Vec::new(); diff --git a/src/language.rs b/src/language.rs index 24ba680..9648fe1 100644 --- a/src/language.rs +++ b/src/language.rs @@ -29,7 +29,9 @@ impl LanguageMetadata { /// /// # Examples /// - /// ```ignore + /// ``` + /// use mdbook_check_code::get_language_metadata; + /// /// let makefile_meta = get_language_metadata("makefile"); /// assert!(makefile_meta.is_complete_filename()); /// assert_eq!(makefile_meta.file_extension, "Makefile"); @@ -38,7 +40,7 @@ impl LanguageMetadata { /// assert!(!c_meta.is_complete_filename()); /// assert_eq!(c_meta.file_extension, ".c"); /// ``` - #[allow(dead_code)] + #[allow(dead_code)] // Public API utility, usage will grow pub fn is_complete_filename(&self) -> bool { !self.file_extension.starts_with('.') } @@ -61,7 +63,9 @@ impl LanguageMetadata { /// /// # Examples /// -/// ```ignore +/// ``` +/// use mdbook_check_code::get_language_metadata; +/// /// let metadata = get_language_metadata("c"); /// assert_eq!(metadata.fence_markers, vec!["c", "h"]); /// assert_eq!(metadata.file_extension, ".c"); diff --git a/src/lib.rs b/src/lib.rs new file mode 100644 index 0000000..b6b60ab --- /dev/null +++ b/src/lib.rs @@ -0,0 +1,29 @@ +//! mdbook-check-code library +//! +//! This library provides the preprocessor implementation for validating code blocks +//! in mdBook projects. The primary interface is the mdbook-check-code binary, but +//! the library can be used programmatically for testing or custom integrations. +//! +//! ## Public API +//! +//! The main public interface is [`CheckCodePreprocessor`], which implements the +//! mdBook `Preprocessor` trait. +//! +//! Additional utilities: +//! - [`get_language_metadata`] - Get metadata for a language (fence markers and file extension) +//! - [`LanguageMetadata`] - Metadata structure for a language + +mod approval; +mod compilation; +mod config; +mod extractor; +mod language; +mod preprocessor; +mod reporting; +mod task_collector; + +// Public API (stable) +pub use preprocessor::CheckCodePreprocessor; + +// Language utilities +pub use language::{get_language_metadata, LanguageMetadata}; diff --git a/src/preprocessor.rs b/src/preprocessor.rs index 8c064e7..ed553fe 100644 --- a/src/preprocessor.rs +++ b/src/preprocessor.rs @@ -35,11 +35,39 @@ use tempfile::TempDir; /// The preprocessor validates compiler paths to prevent command injection attacks. /// Compiler paths cannot contain shell metacharacters (`;`, `|`, `&`, `` ` ``) or /// use parent directory traversal (`..`). -pub struct CheckCodePreprocessor; +pub struct CheckCodePreprocessor { + #[cfg(feature = "test-util")] + skip_approval: bool, +} impl CheckCodePreprocessor { pub fn new() -> Self { - Self + Self { + #[cfg(feature = "test-util")] + skip_approval: false, + } + } + + /// Create preprocessor with approval checking disabled. + /// + /// # Safety + /// + /// **WARNING**: This bypasses SHA256-based security checks and should ONLY + /// be used in tests. This function is only available when the `test-util` + /// feature is enabled. + /// + /// # Example + /// + /// ```toml + /// [dev-dependencies] + /// mdbook-check-code = { version = "0.1", features = ["test-util"] } + /// ``` + #[cfg(feature = "test-util")] + #[allow(dead_code)] // Used by integration tests with test-util feature + pub fn new_for_testing() -> Self { + Self { + skip_approval: true, + } } } @@ -56,10 +84,17 @@ impl CheckCodePreprocessor { Local::now().format("%Y-%m-%d %H:%M:%S") ); - let book_toml_path = ctx.root.join("book.toml"); - if !is_approved(&book_toml_path)? { - reporting::report_approval_error(&book_toml_path)?; - anyhow::bail!("book.toml not approved"); + #[cfg(feature = "test-util")] + let skip_approval = self.skip_approval; + #[cfg(not(feature = "test-util"))] + let skip_approval = false; + + if !skip_approval { + let book_toml_path = ctx.root.join("book.toml"); + if !is_approved(&book_toml_path)? { + reporting::report_approval_error(&book_toml_path)?; + anyhow::bail!("book.toml not approved"); + } } let config = CheckCodeConfig::from_preprocessor_context(ctx)?; @@ -109,7 +144,10 @@ impl Preprocessor for CheckCodePreprocessor { } fn run(&self, ctx: &PreprocessorContext, book: Book) -> Result { - tokio::runtime::Handle::current().block_on(self.run_async(ctx, book)) + // Create a new runtime for preprocessing (mdBook doesn't provide one) + let runtime = tokio::runtime::Runtime::new() + .context("Failed to create Tokio runtime for async preprocessing")?; + runtime.block_on(self.run_async(ctx, book)) } fn supports_renderer(&self, _renderer: &str) -> bool { diff --git a/tests/common/mod.rs b/tests/common/mod.rs new file mode 100644 index 0000000..7a0e8f2 --- /dev/null +++ b/tests/common/mod.rs @@ -0,0 +1,100 @@ +//! Common test utilities for integration tests +//! +//! This module contains shared test fixtures and helper functions used across +//! integration tests. These utilities are not compiled into the library. + +use anyhow::Result; +use mdbook::book::Book; +use mdbook::preprocess::CmdPreprocessor; +use mdbook::MDBook; +use mdbook_check_code::CheckCodePreprocessor; +use std::path::{Path, PathBuf}; +use tempfile::TempDir; + +/// Isolated test fixture with automatic cleanup +/// +/// Creates a temporary copy of a test fixture book, allowing tests to run +/// in parallel without interfering with each other. +pub struct TestFixture { + _book_dir: TempDir, + book_path: PathBuf, +} + +impl TestFixture { + /// Create a new test fixture from the default valid_cases directory + pub fn new() -> Result { + Self::new_from("tests/fixtures/valid_cases") + } + + /// Create a new test fixture from a specific source directory + pub fn new_from(source: impl AsRef) -> Result { + let book_dir = TempDir::new()?; + + // Copy fixture to temp location + copy_dir_all(source.as_ref(), book_dir.path())?; + + Ok(Self { + book_path: book_dir.path().to_path_buf(), + _book_dir: book_dir, + }) + } + + /// Get the path to the book directory + pub fn book_path(&self) -> &Path { + &self.book_path + } +} + +/// Helper to run preprocessor on a test book +/// +/// Wraps an MDBook instance and provides a convenient async run method +/// that simulates how mdBook would invoke the preprocessor. +pub struct PreprocessorTest { + book: MDBook, +} + +impl PreprocessorTest { + /// Create a preprocessor test from a fixture + pub fn from_fixture(fixture: &TestFixture) -> Result { + let book = MDBook::load(fixture.book_path())?; + Ok(Self { book }) + } + + /// Run the preprocessor on the test book + /// + /// Uses `CheckCodePreprocessor::new_for_testing()` to bypass approval checks, + /// allowing tests to run without manual approval. + pub async fn run(&self) -> Result { + // Create JSON input like mdbook would send + let input_json = serde_json::json!([ + { + "root": self.book.root, + "config": self.book.config, + "renderer": "html", + "mdbook_version": env!("CARGO_PKG_VERSION"), + }, + self.book.book + ]); + + let input_str = serde_json::to_string(&input_json)?; + let (ctx, book) = CmdPreprocessor::parse_input(input_str.as_bytes())?; + + let preprocessor = CheckCodePreprocessor::new_for_testing(); + preprocessor.run_async(&ctx, book).await + } +} + +/// Recursively copy all files and directories from src to dst +fn copy_dir_all(src: impl AsRef, dst: impl AsRef) -> Result<()> { + std::fs::create_dir_all(&dst)?; + for entry in std::fs::read_dir(src)? { + let entry = entry?; + let ty = entry.file_type()?; + if ty.is_dir() { + copy_dir_all(entry.path(), dst.as_ref().join(entry.file_name()))?; + } else { + std::fs::copy(entry.path(), dst.as_ref().join(entry.file_name()))?; + } + } + Ok(()) +} diff --git a/tests/fixtures/error_cases/book.toml b/tests/fixtures/error_cases/book.toml new file mode 100644 index 0000000..5cfae4e --- /dev/null +++ b/tests/fixtures/error_cases/book.toml @@ -0,0 +1,17 @@ +[book] +authors = ["Test Author"] +language = "en" +multilingual = false +src = "src" +title = "Error Test Book" + +[preprocessor.check-code] +command = "../../../target/release/mdbook-check-code" + +# C configuration +[preprocessor.check-code.languages.c] +enabled = true +compiler = "gcc" +flags = ["-fsyntax-only"] + +[output.html] diff --git a/tests/fixtures/error_cases/src/SUMMARY.md b/tests/fixtures/error_cases/src/SUMMARY.md new file mode 100644 index 0000000..72f300d --- /dev/null +++ b/tests/fixtures/error_cases/src/SUMMARY.md @@ -0,0 +1,3 @@ +# Summary + +- [Test Error](test_error.md) diff --git a/tests/fixtures/error_cases/src/test_error.md b/tests/fixtures/error_cases/src/test_error.md new file mode 100644 index 0000000..6a44d38 --- /dev/null +++ b/tests/fixtures/error_cases/src/test_error.md @@ -0,0 +1,12 @@ +# Test Error + +This file contains invalid code to test error reporting. + +## Invalid C code + +```c +int main() { + THIS IS INVALID C CODE!!! + return 0; +} +``` diff --git a/tests/fixtures/book.toml b/tests/fixtures/valid_cases/book.toml similarity index 100% rename from tests/fixtures/book.toml rename to tests/fixtures/valid_cases/book.toml diff --git a/tests/fixtures/src/SUMMARY.md b/tests/fixtures/valid_cases/src/SUMMARY.md similarity index 100% rename from tests/fixtures/src/SUMMARY.md rename to tests/fixtures/valid_cases/src/SUMMARY.md diff --git a/tests/fixtures/src/intro.md b/tests/fixtures/valid_cases/src/intro.md similarity index 100% rename from tests/fixtures/src/intro.md rename to tests/fixtures/valid_cases/src/intro.md diff --git a/tests/fixtures/src/other_langs/README.md b/tests/fixtures/valid_cases/src/other_langs/README.md similarity index 100% rename from tests/fixtures/src/other_langs/README.md rename to tests/fixtures/valid_cases/src/other_langs/README.md diff --git a/tests/fixtures/src/other_langs/c_examples/README.md b/tests/fixtures/valid_cases/src/other_langs/c_examples/README.md similarity index 100% rename from tests/fixtures/src/other_langs/c_examples/README.md rename to tests/fixtures/valid_cases/src/other_langs/c_examples/README.md diff --git a/tests/fixtures/src/other_langs/c_examples/c_example.md b/tests/fixtures/valid_cases/src/other_langs/c_examples/c_example.md similarity index 100% rename from tests/fixtures/src/other_langs/c_examples/c_example.md rename to tests/fixtures/valid_cases/src/other_langs/c_examples/c_example.md diff --git a/tests/fixtures/src/other_langs/ts_examples/README.md b/tests/fixtures/valid_cases/src/other_langs/ts_examples/README.md similarity index 100% rename from tests/fixtures/src/other_langs/ts_examples/README.md rename to tests/fixtures/valid_cases/src/other_langs/ts_examples/README.md diff --git a/tests/fixtures/src/other_langs/ts_examples/typescript_example.md b/tests/fixtures/valid_cases/src/other_langs/ts_examples/typescript_example.md similarity index 100% rename from tests/fixtures/src/other_langs/ts_examples/typescript_example.md rename to tests/fixtures/valid_cases/src/other_langs/ts_examples/typescript_example.md diff --git a/tests/fixtures/src/parasol_examples/README.md b/tests/fixtures/valid_cases/src/parasol_examples/README.md similarity index 100% rename from tests/fixtures/src/parasol_examples/README.md rename to tests/fixtures/valid_cases/src/parasol_examples/README.md diff --git a/tests/fixtures/src/parasol_examples/basic.md b/tests/fixtures/valid_cases/src/parasol_examples/basic.md similarity index 100% rename from tests/fixtures/src/parasol_examples/basic.md rename to tests/fixtures/valid_cases/src/parasol_examples/basic.md diff --git a/tests/fixtures/src/parasol_examples/propagate.md b/tests/fixtures/valid_cases/src/parasol_examples/propagate.md similarity index 100% rename from tests/fixtures/src/parasol_examples/propagate.md rename to tests/fixtures/valid_cases/src/parasol_examples/propagate.md diff --git a/tests/fixtures/src/solidity_example.md b/tests/fixtures/valid_cases/src/solidity_example.md similarity index 100% rename from tests/fixtures/src/solidity_example.md rename to tests/fixtures/valid_cases/src/solidity_example.md diff --git a/tests/fixtures/src/test_error.md b/tests/fixtures/valid_cases/src/test_error.md similarity index 100% rename from tests/fixtures/src/test_error.md rename to tests/fixtures/valid_cases/src/test_error.md diff --git a/tests/integration.rs b/tests/integration.rs new file mode 100644 index 0000000..6f2a296 --- /dev/null +++ b/tests/integration.rs @@ -0,0 +1,165 @@ +//! Integration tests for mdbook-check-code +//! +//! These tests verify the full end-to-end workflow by running the +//! preprocessor against test fixtures in isolated environments. +//! +//! ## Test Architecture +//! +//! Each test uses `TestFixture` to create an isolated environment with: +//! - Temporary book directory (copy of fixtures) +//! - Automatic cleanup via RAII (Drop trait) +//! +//! Tests use `CheckCodePreprocessor::new_for_testing()` to bypass approval checks, +//! allowing fully parallel execution without environment variable manipulation. +//! +//! ## Adding New Tests +//! +//! 1. Create a new fixture in tests/fixtures/ if needed +//! 2. Use `TestFixture::new()` or `TestFixture::new_from()` +//! 3. Use `#[tokio::test]` for async tests +//! 4. Assert on the returned Result or Book + +mod common; + +use anyhow::Result; +use common::{PreprocessorTest, TestFixture}; +use mdbook::preprocess::CmdPreprocessor; +use mdbook::MDBook; +use mdbook_check_code::CheckCodePreprocessor; + +// ===== Tests ===== + +#[tokio::test] +async fn integration_valid_code_blocks_compile_successfully() -> Result<()> { + let fixture = TestFixture::new()?; + let test = PreprocessorTest::from_fixture(&fixture)?; + + let result = test.run().await; + + assert!( + result.is_ok(), + "Valid code should compile: {:?}", + result.err() + ); + Ok(()) +} + +#[tokio::test] +async fn integration_multiple_languages_supported() -> Result<()> { + let fixture = TestFixture::new()?; + let test = PreprocessorTest::from_fixture(&fixture)?; + + // Fixture contains C, TypeScript, and Solidity + let result = test.run().await; + + assert!( + result.is_ok(), + "Multi-language support failed: {:?}", + result.err() + ); + Ok(()) +} + +#[tokio::test] +async fn integration_compilation_errors_detected() -> Result<()> { + let fixture = TestFixture::new_from("tests/fixtures/error_cases")?; + let test = PreprocessorTest::from_fixture(&fixture)?; + + let result = test.run().await; + + assert!(result.is_err(), "Invalid code should fail compilation"); + + if let Err(e) = result { + let error_msg = format!("{:#}", e); + assert!( + error_msg.contains("compilation"), + "Unexpected error: {}", + error_msg + ); + } + + Ok(()) +} + +#[tokio::test] +async fn integration_book_structure_unchanged() -> Result<()> { + let fixture = TestFixture::new()?; + let md = MDBook::load(fixture.book_path())?; + let original_sections = md.book.sections.len(); + + let test = PreprocessorTest::from_fixture(&fixture)?; + let result_book = test.run().await?; + + assert_eq!( + result_book.sections.len(), + original_sections, + "Preprocessor should not modify book structure" + ); + + Ok(()) +} + +#[tokio::test] +async fn integration_nested_chapters_processed() -> Result<()> { + let fixture = TestFixture::new()?; + let md = MDBook::load(fixture.book_path())?; + + // Verify nested structure exists: + // - parasol_examples/ + // - other_langs/c_examples/ + // - other_langs/ts_examples/ + + let has_nested = md.book.iter().any(|item| { + if let mdbook::book::BookItem::Chapter(ch) = item { + ch.path + .as_ref() + .is_some_and(|p| p.to_str().is_some_and(|s| s.contains('/'))) + } else { + false + } + }); + + assert!(has_nested, "Fixture should contain nested chapters"); + + let test = PreprocessorTest::from_fixture(&fixture)?; + let result = test.run().await; + + assert!(result.is_ok(), "Nested chapters failed: {:?}", result.err()); + Ok(()) +} + +#[tokio::test] +async fn integration_unapproved_book_rejected() -> Result<()> { + let fixture = TestFixture::new()?; + let md = MDBook::load(fixture.book_path())?; + + let input_json = serde_json::json!([ + { + "root": md.root, + "config": md.config, + "renderer": "html", + "mdbook_version": env!("CARGO_PKG_VERSION"), + }, + md.book + ]); + + let input_str = serde_json::to_string(&input_json)?; + let (ctx, book) = CmdPreprocessor::parse_input(input_str.as_bytes())?; + + // Use regular preprocessor (NOT new_for_testing) to check approval + let preprocessor = CheckCodePreprocessor::new(); + let result = preprocessor.run_async(&ctx, book).await; + + assert!(result.is_err(), "Unapproved book should be rejected"); + + if let Err(e) = result { + let error_msg = format!("{:#}", e); + assert!( + error_msg.contains("not approved"), + "Wrong error: {}", + error_msg + ); + } + + Ok(()) +} From 4489d41cabcf45de8a3f3b9d9b49f599b5f0f192 Mon Sep 17 00:00:00 2001 From: Ryan Orendorff <12442942+ryanorendorff@users.noreply.github.com> Date: Fri, 17 Oct 2025 10:47:20 -0600 Subject: [PATCH 2/6] review changes --- Cargo.toml | 5 +++++ src/approval.rs | 6 +++--- src/language.rs | 2 +- src/lib.rs | 5 +---- src/preprocessor.rs | 8 ++------ src/reporting.rs | 31 ++++++++++++++++++++----------- tests/common/mod.rs | 13 +++++-------- tests/integration.rs | 14 +++++++------- 8 files changed, 44 insertions(+), 40 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 27c9abc..49c7269 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,11 @@ path = "src/lib.rs" name = "mdbook-check-code" path = "src/main.rs" +[[test]] +name = "integration" +path = "tests/integration.rs" +required-features = ["test-util"] + [dependencies] mdbook = "0.4" pulldown-cmark = "0.11" diff --git a/src/approval.rs b/src/approval.rs index 37e0994..fab4bc0 100644 --- a/src/approval.rs +++ b/src/approval.rs @@ -39,7 +39,7 @@ pub fn is_approved(book_toml_path: &Path) -> Result { } /// Approve a book.toml -#[allow(dead_code)] // Used by CLI binary (main.rs), not library +#[allow(dead_code)] // Used by CLI binary pub fn approve(book_toml_path: &Path) -> Result<()> { let content = fs::read_to_string(book_toml_path) .with_context(|| format!("Failed to read {}", book_toml_path.display()))?; @@ -66,7 +66,7 @@ pub fn approve(book_toml_path: &Path) -> Result<()> { } /// Deny (remove approval) for a book.toml -#[allow(dead_code)] // Used by CLI binary (main.rs), not library +#[allow(dead_code)] // Used by CLI binary pub fn deny(book_toml_path: &Path) -> Result<()> { let content = fs::read_to_string(book_toml_path) .with_context(|| format!("Failed to read {}", book_toml_path.display()))?; @@ -87,7 +87,7 @@ pub fn deny(book_toml_path: &Path) -> Result<()> { } /// List all approved books -#[allow(dead_code)] // Used by CLI binary (main.rs), not library +#[allow(dead_code)] // Used by CLI binary pub fn list_approved() -> Result> { let approval_dir = get_approval_dir()?; diff --git a/src/language.rs b/src/language.rs index 9648fe1..69dbb6c 100644 --- a/src/language.rs +++ b/src/language.rs @@ -40,7 +40,7 @@ impl LanguageMetadata { /// assert!(!c_meta.is_complete_filename()); /// assert_eq!(c_meta.file_extension, ".c"); /// ``` - #[allow(dead_code)] // Public API utility, usage will grow + #[allow(dead_code)] pub fn is_complete_filename(&self) -> bool { !self.file_extension.starts_with('.') } diff --git a/src/lib.rs b/src/lib.rs index b6b60ab..9f1c8e7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,8 +22,5 @@ mod preprocessor; mod reporting; mod task_collector; -// Public API (stable) -pub use preprocessor::CheckCodePreprocessor; - -// Language utilities pub use language::{get_language_metadata, LanguageMetadata}; +pub use preprocessor::CheckCodePreprocessor; diff --git a/src/preprocessor.rs b/src/preprocessor.rs index ed553fe..94be3eb 100644 --- a/src/preprocessor.rs +++ b/src/preprocessor.rs @@ -1,9 +1,9 @@ use crate::approval::is_approved; use crate::config::CheckCodeConfig; use crate::language::LanguageRegistry; +use crate::reporting::print_info; use crate::{compilation, reporting, task_collector}; use anyhow::{Context, Result}; -use chrono::Local; use mdbook::book::Book; use mdbook::preprocess::{Preprocessor, PreprocessorContext}; use tempfile::TempDir; @@ -79,10 +79,7 @@ impl Default for CheckCodePreprocessor { impl CheckCodePreprocessor { pub async fn run_async(&self, ctx: &PreprocessorContext, mut book: Book) -> Result { - eprintln!( - "{} [INFO] (mdbook_check_code): Preprocessor started", - Local::now().format("%Y-%m-%d %H:%M:%S") - ); + print_info("Preprocessor started"); #[cfg(feature = "test-util")] let skip_approval = self.skip_approval; @@ -144,7 +141,6 @@ impl Preprocessor for CheckCodePreprocessor { } fn run(&self, ctx: &PreprocessorContext, book: Book) -> Result { - // Create a new runtime for preprocessing (mdBook doesn't provide one) let runtime = tokio::runtime::Runtime::new() .context("Failed to create Tokio runtime for async preprocessing")?; runtime.block_on(self.run_async(ctx, book)) diff --git a/src/reporting.rs b/src/reporting.rs index f4b2148..a99f2f3 100644 --- a/src/reporting.rs +++ b/src/reporting.rs @@ -6,15 +6,26 @@ use std::fmt::Display; use std::path::Path; use std::time::Duration; -/// Prints an error message to stderr with mdBook-style timestamp and prefix. -pub fn print_error(message: S) { +/// Internal helper for printing messages with consistent formatting. +fn print_message(level: &str, message: S) { eprintln!( - "{} [ERROR] (mdbook_check_code): {}", + "{} [{}] (mdbook_check_code): {}", Local::now().format("%Y-%m-%d %H:%M:%S"), + level, message ); } +/// Prints an error message to stderr with mdBook-style timestamp and prefix. +pub fn print_error(message: S) { + print_message("ERROR", message); +} + +/// Prints an info message to stderr with mdBook-style timestamp and prefix. +pub fn print_info(message: S) { + print_message("INFO", message); +} + /// Reports the approval error to stderr with mdBook-style formatting. pub fn report_approval_error(book_toml_path: &Path) -> Result<()> { print_error("book.toml not approved for code execution"); @@ -110,18 +121,16 @@ pub fn print_compilation_statistics(results: &[CompilationResult], parallel_dura }; let parallel_ms = parallel_duration.as_millis(); - eprintln!( - "{} [INFO] (mdbook_check_code): Successfully validated {} code block(s) ({})", - Local::now().format("%Y-%m-%d %H:%M:%S"), + print_info(format!( + "Successfully validated {} code block(s) ({})", total_blocks, stats_str - ); - eprintln!( - "{} [INFO] (mdbook_check_code): Preprocessor finished in {}ms (avg {}ms per block)", - Local::now().format("%Y-%m-%d %H:%M:%S"), + )); + print_info(format!( + "Preprocessor finished in {}ms (avg {}ms per block)", parallel_ms, avg_ms - ); + )); log::debug!("Timing breakdown by language:"); for (lang, count) in sorted_stats { diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 7a0e8f2..3610b6e 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -21,21 +21,18 @@ pub struct TestFixture { } impl TestFixture { - /// Create a new test fixture from the default valid_cases directory - pub fn new() -> Result { - Self::new_from("tests/fixtures/valid_cases") - } - - /// Create a new test fixture from a specific source directory - pub fn new_from(source: impl AsRef) -> Result { + /// Create a new test fixture from a source directory + pub fn new(source: impl AsRef) -> Result { let book_dir = TempDir::new()?; // Copy fixture to temp location copy_dir_all(source.as_ref(), book_dir.path())?; + let book_path = book_dir.path().to_path_buf(); + Ok(Self { - book_path: book_dir.path().to_path_buf(), _book_dir: book_dir, + book_path, }) } diff --git a/tests/integration.rs b/tests/integration.rs index 6f2a296..f5349af 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -15,7 +15,7 @@ //! ## Adding New Tests //! //! 1. Create a new fixture in tests/fixtures/ if needed -//! 2. Use `TestFixture::new()` or `TestFixture::new_from()` +//! 2. Use `TestFixture::new("path/to/fixture")` //! 3. Use `#[tokio::test]` for async tests //! 4. Assert on the returned Result or Book @@ -31,7 +31,7 @@ use mdbook_check_code::CheckCodePreprocessor; #[tokio::test] async fn integration_valid_code_blocks_compile_successfully() -> Result<()> { - let fixture = TestFixture::new()?; + let fixture = TestFixture::new("tests/fixtures/valid_cases")?; let test = PreprocessorTest::from_fixture(&fixture)?; let result = test.run().await; @@ -46,7 +46,7 @@ async fn integration_valid_code_blocks_compile_successfully() -> Result<()> { #[tokio::test] async fn integration_multiple_languages_supported() -> Result<()> { - let fixture = TestFixture::new()?; + let fixture = TestFixture::new("tests/fixtures/valid_cases")?; let test = PreprocessorTest::from_fixture(&fixture)?; // Fixture contains C, TypeScript, and Solidity @@ -62,7 +62,7 @@ async fn integration_multiple_languages_supported() -> Result<()> { #[tokio::test] async fn integration_compilation_errors_detected() -> Result<()> { - let fixture = TestFixture::new_from("tests/fixtures/error_cases")?; + let fixture = TestFixture::new("tests/fixtures/error_cases")?; let test = PreprocessorTest::from_fixture(&fixture)?; let result = test.run().await; @@ -83,7 +83,7 @@ async fn integration_compilation_errors_detected() -> Result<()> { #[tokio::test] async fn integration_book_structure_unchanged() -> Result<()> { - let fixture = TestFixture::new()?; + let fixture = TestFixture::new("tests/fixtures/valid_cases")?; let md = MDBook::load(fixture.book_path())?; let original_sections = md.book.sections.len(); @@ -101,7 +101,7 @@ async fn integration_book_structure_unchanged() -> Result<()> { #[tokio::test] async fn integration_nested_chapters_processed() -> Result<()> { - let fixture = TestFixture::new()?; + let fixture = TestFixture::new("tests/fixtures/valid_cases")?; let md = MDBook::load(fixture.book_path())?; // Verify nested structure exists: @@ -130,7 +130,7 @@ async fn integration_nested_chapters_processed() -> Result<()> { #[tokio::test] async fn integration_unapproved_book_rejected() -> Result<()> { - let fixture = TestFixture::new()?; + let fixture = TestFixture::new("tests/fixtures/valid_cases")?; let md = MDBook::load(fixture.book_path())?; let input_json = serde_json::json!([ From dbfa6d56899633b5945fb169a7721b66f0a7c91b Mon Sep 17 00:00:00 2001 From: Ryan Orendorff <12442942+ryanorendorff@users.noreply.github.com> Date: Fri, 17 Oct 2025 10:50:36 -0600 Subject: [PATCH 3/6] Rename flag to integration-tests --- Cargo.toml | 4 ++-- src/preprocessor.rs | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 49c7269..bf4f25d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ path = "src/main.rs" [[test]] name = "integration" path = "tests/integration.rs" -required-features = ["test-util"] +required-features = ["integration-tests"] [dependencies] mdbook = "0.4" @@ -40,4 +40,4 @@ tokio = { version = "1.42", features = ["macros", "rt-multi-thread"] } [features] default = [] -test-util = [] +integration-tests = [] diff --git a/src/preprocessor.rs b/src/preprocessor.rs index 94be3eb..edf7146 100644 --- a/src/preprocessor.rs +++ b/src/preprocessor.rs @@ -36,14 +36,14 @@ use tempfile::TempDir; /// Compiler paths cannot contain shell metacharacters (`;`, `|`, `&`, `` ` ``) or /// use parent directory traversal (`..`). pub struct CheckCodePreprocessor { - #[cfg(feature = "test-util")] + #[cfg(feature = "integration-tests")] skip_approval: bool, } impl CheckCodePreprocessor { pub fn new() -> Self { Self { - #[cfg(feature = "test-util")] + #[cfg(feature = "integration-tests")] skip_approval: false, } } @@ -53,17 +53,17 @@ impl CheckCodePreprocessor { /// # Safety /// /// **WARNING**: This bypasses SHA256-based security checks and should ONLY - /// be used in tests. This function is only available when the `test-util` + /// be used in tests. This function is only available when the `integration-tests` /// feature is enabled. /// /// # Example /// /// ```toml /// [dev-dependencies] - /// mdbook-check-code = { version = "0.1", features = ["test-util"] } + /// mdbook-check-code = { version = "0.1", features = ["integration-tests"] } /// ``` - #[cfg(feature = "test-util")] - #[allow(dead_code)] // Used by integration tests with test-util feature + #[cfg(feature = "integration-tests")] + #[allow(dead_code)] // Used by integration tests with integration-tests feature pub fn new_for_testing() -> Self { Self { skip_approval: true, @@ -81,9 +81,9 @@ impl CheckCodePreprocessor { pub async fn run_async(&self, ctx: &PreprocessorContext, mut book: Book) -> Result { print_info("Preprocessor started"); - #[cfg(feature = "test-util")] + #[cfg(feature = "integration-tests")] let skip_approval = self.skip_approval; - #[cfg(not(feature = "test-util"))] + #[cfg(not(feature = "integration-tests"))] let skip_approval = false; if !skip_approval { From 6f54957af0af034a52bec2698ac5edbf9fff25f2 Mon Sep 17 00:00:00 2001 From: Ryan Orendorff <12442942+ryanorendorff@users.noreply.github.com> Date: Fri, 17 Oct 2025 10:58:42 -0600 Subject: [PATCH 4/6] fmt --- src/reporting.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/reporting.rs b/src/reporting.rs index a99f2f3..680fa80 100644 --- a/src/reporting.rs +++ b/src/reporting.rs @@ -123,13 +123,11 @@ pub fn print_compilation_statistics(results: &[CompilationResult], parallel_dura print_info(format!( "Successfully validated {} code block(s) ({})", - total_blocks, - stats_str + total_blocks, stats_str )); print_info(format!( "Preprocessor finished in {}ms (avg {}ms per block)", - parallel_ms, - avg_ms + parallel_ms, avg_ms )); log::debug!("Timing breakdown by language:"); From 341045a998ee5ad86b443a715d4775f677432e62 Mon Sep 17 00:00:00 2001 From: Ryan Orendorff <12442942+ryanorendorff@users.noreply.github.com> Date: Fri, 17 Oct 2025 11:57:54 -0600 Subject: [PATCH 5/6] fix(nix): simplify tests to use cargo test infrastructure Replace complex manual mdbook invocation with craneLib.cargoTest that runs `cargo test --features integration-tests`. This single check runs all tests (unit + integration) and properly handles both valid_cases and error_cases fixtures. Renamed from runE2ETests to mdbook-check-code-test for consistency with other check names. --- flake.nix | 43 +++++++++---------------------------------- 1 file changed, 9 insertions(+), 34 deletions(-) diff --git a/flake.nix b/flake.nix index a75916c..3d9938f 100644 --- a/flake.nix +++ b/flake.nix @@ -40,8 +40,6 @@ }; cargoArtifacts = craneLib.buildDepsOnly commonArgs; - fixture-src = gitignoreSource ./tests/fixtures; - # Map script names to their specific dependencies scriptDeps = { format-markdown = with pkgs; [ git nodePackages.prettier ]; @@ -104,12 +102,13 @@ echo "Markdown formatting check passed" > $out/result ''; - # The script inlined for brevity, consider extracting it - # so that it becomes independent of nix - runE2ETests = pkgs.runCommand "e2e-tests" { + # Run all tests including integration tests + # Use gitignoreSource to include test fixtures (cleanCargoSource filters them out) + mdbook-check-code-test = craneLib.cargoTest (commonArgs // { + src = gitignoreSource ./.; + inherit cargoArtifacts; + cargoTestExtraArgs = "--features integration-tests"; nativeBuildInputs = with pkgs; [ - mdbook - # C compilers sunscreen-llvm-pkg gcc @@ -119,33 +118,9 @@ nodePackages.typescript solc ]; - } '' - cp -r ${fixture-src}/* $TMPDIR/ - - # Make everything in this directory writable, otherwise all the - # commands below will fail. - chmod -R u+w . - - export CLANG="${sunscreen-llvm-pkg}/bin/clang" - export RUST_LOG=info - - # Set XDG_DATA_HOME to a temporary location for approval storage - # This allows the approval mechanism to work in the nix sandbox - export XDG_DATA_HOME=$TMPDIR/xdg-data - - # Replace the mdbook-check-code path in book.toml - # to point to the built binary in this derivation. - sed -i "s|../../target/release/mdbook-check-code|${mdbook-check-code}/bin/mdbook-check-code|g" book.toml - - # Approve the book.toml for security - ${mdbook-check-code}/bin/mdbook-check-code allow - - mdbook build - - # After the build is successful, copy the final output to the expected $out path. - mkdir $out - cp -r $TMPDIR/book/* $out - ''; + CLANG = "${sunscreen-llvm-pkg}/bin/clang"; + RUST_LOG = "info"; + }); }; devShells.default = with pkgs; From e88b4a79e8ae8386b322d6509f6450d9a24418ce Mon Sep 17 00:00:00 2001 From: Ryan Orendorff <12442942+ryanorendorff@users.noreply.github.com> Date: Fri, 17 Oct 2025 12:04:48 -0600 Subject: [PATCH 6/6] fix(ci): update integration test check name Update workflow to use new check name mdbook-check-code-test instead of runE2ETests. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 47416b1..fab9cfc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -98,4 +98,4 @@ jobs: uses: DeterminateSystems/magic-nix-cache-action@v8 - name: Run integration tests - run: nix build -L .#checks.x86_64-linux.runE2ETests + run: nix build -L .#checks.x86_64-linux.mdbook-check-code-test