Conversation
Reviewer's GuideThis PR restructures the integration test suite by splitting the monolithic tests into feature-focused modules, centralizes common test macros and fixtures under tests/common with a shared prelude, updates documentation to use unicode ellipsis in rstest macros, and removes the old integration.rs file with a small clippy annotation tweak in main.rs. Class diagram for the new integration test module structureclassDiagram
class common {
<<module>>
+shared helpers
+test macros
}
class prelude {
<<module>>
+re-exports from common
}
class lists {
<<test module>>
+list renumbering tests
}
class table {
<<test module>>
+table reflow and HTML conversion tests
}
class wrap {
<<test module>>
+wrapping behavior tests
}
class cli {
<<test module>>
+CLI options tests
}
class breaks {
<<test module>>
+thematic break formatting tests
}
common <|-- prelude
prelude <.. lists : uses
prelude <.. table : uses
prelude <.. wrap : uses
prelude <.. cli : uses
prelude <.. breaks : uses
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughRefactor the integration test suite by splitting it into focused modules for tables, lists, wrapping, breaks, and CLI behaviour. Remove the monolithic Changes
Sequence Diagram(s)sequenceDiagram
participant Tester as Test Module (e.g. table.rs, lists.rs)
participant CLI as mdtablefix CLI
participant Core as Core Functions (e.g. reflow_table, renumber_lists)
participant Utils as Shared Test Utilities
Tester->>Utils: Import macros/functions via prelude
Tester->>CLI: Invoke CLI with options (e.g. --wrap, --renumber)
CLI->>Core: Call core processing functions
Core-->>CLI: Return processed output
CLI-->>Tester: Output for assertions
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rsInstructions used from: Sources:
⚙️ CodeRabbit Configuration File 🧬 Code Graph Analysis (1)tests/wrap.rs (2)
🔇 Additional comments (26)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Many of the renumbering tests are nearly identical—consider converting them into a single parameterized rstest to cut down on boilerplate and make it easier to add new cases.
- I noticed some tests still use manual include_str! + .lines().map(...) to build Vec—for consistency you could switch those to the include_lines! macro from tests/common.
- Every file currently starts with
#[macro_use] mod common; use assert_cmd::Command;—you might introduce a small test-prelude module that re-exports common macros and Command to simplify those imports.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Many of the renumbering tests are nearly identical—consider converting them into a single parameterized rstest to cut down on boilerplate and make it easier to add new cases.
- I noticed some tests still use manual include_str! + .lines().map(...) to build Vec<String>—for consistency you could switch those to the include_lines! macro from tests/common.
- Every file currently starts with `#[macro_use] mod common; use assert_cmd::Command;`—you might introduce a small test-prelude module that re-exports common macros and Command to simplify those imports.
## Individual Comments
### Comment 1
<location> `tests/table.rs:212` </location>
<code_context>
+#[macro_use]
+mod common;
+
+#[test]
+fn test_format_breaks_basic() {
+ let input = lines_vec!["foo", "***", "bar"];
</code_context>
<issue_to_address>
Consider adding a test for input with only whitespace or empty lines.
Please add a test to verify that process_stream handles whitespace-only or empty line input without errors or unintended changes.
</issue_to_address>
### Comment 2
<location> `tests/wrap.rs:14` </location>
<code_context>
-///
-/// Ensures that a single long paragraph is split into multiple lines, each not exceeding 80
-/// characters.
-fn test_wrap_paragraph() {
- let input = lines_vec![
- "This is a very long paragraph that should be wrapped at eighty columns so it needs to \
- contain enough words to exceed that limit.",
- ];
- let output = process_stream(&input);
- assert!(output.len() > 1);
- assert!(output.iter().all(|l| l.len() <= 80));
-}
-
</code_context>
<issue_to_address>
Add a test for paragraphs with very long words (no spaces).
Please add a test case with a single word exceeding 80 characters to verify correct handling by the wrapping logic.
Suggested implementation:
```rust
fn test_wrap_paragraph() {
let input = lines_vec![
```
```rust
fn test_wrap_paragraph() {
let input = lines_vec![
"This is a very long paragraph that should be wrapped at eighty columns so it needs to \
contain enough words to exceed that limit.",
];
let output = process_stream(&input);
assert!(output.len() > 1);
assert!(output.iter().all(|l| l.len() <= 80));
}
///
/// Ensures that a paragraph with a single word longer than 80 characters is handled correctly.
#[test]
fn test_wrap_paragraph_with_long_word() {
let long_word = "a".repeat(100);
let input = lines_vec![&long_word];
let output = process_stream(&input);
// The output should contain the long word, possibly unbroken, and not panic or error.
assert_eq!(output.len(), 1);
assert_eq!(output[0], long_word);
}
```
</issue_to_address>
### Comment 3
<location> `tests/wrap.rs:204` </location>
<code_context>
-}
-
-#[test]
-fn test_wrap_blockquote_nested() {
- let input = lines_vec![concat!(
- "> > This nested quote contains enough text to require wrapping so that we ",
- "can verify multi-level handling."
- )];
- let output = process_stream(&input);
- common::assert_wrapped_blockquote(&output, "> > ", 2);
- let joined = output
- .iter()
- .map(|l| l.trim_start_matches("> > "))
- .collect::<Vec<_>>()
- .join(" ");
- assert_eq!(joined, input[0].trim_start_matches("> > "));
-}
-
</code_context>
<issue_to_address>
Consider adding a test for blockquotes with mixed indentation (spaces and tabs).
Add a test case with blockquotes using both spaces and tabs in the prefix to verify correct handling of mixed indentation.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
}
=======
}
#[test]
fn test_wrap_blockquote_mixed_indentation() {
// This test checks blockquotes with mixed spaces and tabs in the prefix
let input = lines_vec![
"> \t> \tThis blockquote uses both spaces and tabs in the prefix to test mixed indentation handling."
];
let output = process_stream(&input);
// The expected prefix is "> \t> \t"
common::assert_wrapped_blockquote(&output, "> \t> \t", 2);
let joined = output
.iter()
.map(|l| l.trim_start_matches("> \t> \t"))
.collect::<Vec<_>>()
.join(" ");
assert_eq!(joined, input[0].trim_start_matches("> \t> \t"));
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `tests/cli.rs:101` </location>
<code_context>
+}
+
+#[test]
+fn test_cli_ellipsis_long_sequence() {
+ let output = Command::cargo_bin("mdtablefix")
+ .unwrap()
+ .arg("--ellipsis")
+ .write_stdin("wait....\n")
+ .output()
+ .unwrap();
+ assert!(output.status.success());
+ assert_eq!(String::from_utf8_lossy(&output.stdout), "wait….\n");
+}
</code_context>
<issue_to_address>
Consider adding a test for ellipsis replacement in a line with multiple ellipsis sequences.
Add a test case with a line containing multiple '...' sequences to verify correct handling by the --ellipsis option.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
#[test]
fn test_cli_ellipsis_long_sequence() {
let output = Command::cargo_bin("mdtablefix")
.unwrap()
.arg("--ellipsis")
.write_stdin("wait....\n")
.output()
.unwrap();
assert!(output.status.success());
assert_eq!(String::from_utf8_lossy(&output.stdout), "wait….\n");
}
=======
#[test]
fn test_cli_ellipsis_long_sequence() {
let output = Command::cargo_bin("mdtablefix")
.unwrap()
.arg("--ellipsis")
.write_stdin("wait....\n")
.output()
.unwrap();
assert!(output.status.success());
assert_eq!(String::from_utf8_lossy(&output.stdout), "wait….\n");
}
#[test]
fn test_cli_ellipsis_multiple_sequences() {
let output = Command::cargo_bin("mdtablefix")
.unwrap()
.arg("--ellipsis")
.write_stdin("First... then second... done.\n")
.output()
.unwrap();
assert!(output.status.success());
assert_eq!(
String::from_utf8_lossy(&output.stdout),
"First… then second… done.\n"
);
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 5
<location> `tests/breaks.rs:39` </location>
<code_context>
+}
+
+#[test]
+fn test_format_breaks_with_spaces_and_indent() {
+ let input = lines_vec![" - - - "];
+ let expected: Vec<Cow<str>> = vec![Cow::Owned("_".repeat(THEMATIC_BREAK_LEN))];
+ assert_eq!(format_breaks(&input), expected);
+}
+
</code_context>
<issue_to_address>
Consider adding a test for thematic breaks with mixed characters and excessive length.
Please add a test case for thematic breaks that use a mix of valid characters and exceed the standard length, such as '***---___'.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
#[test]
fn test_format_breaks_with_spaces_and_indent() {
let input = lines_vec![" - - - "];
let expected: Vec<Cow<str>> = vec![Cow::Owned("_".repeat(THEMATIC_BREAK_LEN))];
assert_eq!(format_breaks(&input), expected);
}
=======
#[test]
fn test_format_breaks_with_spaces_and_indent() {
let input = lines_vec![" - - - "];
let expected: Vec<Cow<str>> = vec![Cow::Owned("_".repeat(THEMATIC_BREAK_LEN))];
assert_eq!(format_breaks(&input), expected);
}
#[test]
fn test_format_breaks_mixed_chars_excessive_length() {
let input = lines_vec!["***---___"];
let expected: Vec<Cow<str>> = vec![Cow::Owned("_".repeat(THEMATIC_BREAK_LEN))];
assert_eq!(format_breaks(&input), expected);
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 6
<location> `docs/rust-testing-with-rstest-fixtures.md:1359` </location>
<code_context>
+| #[timeout(…)] | Sets a timeout for an asynchronous test. |
+| #[files("glob_pattern",…)] | Injects file paths (or contents, with mode=) matching a glob pattern as test arguments. |
By mastering `rstest`, Rust developers can significantly elevate the quality
and efficiency of their testing practices, leading to more reliable and
--- a/tests/common/mod.rs
</code_context>
<issue_to_address>
The phrase uses 'By mastering', which implies 'you' and should be reworded to avoid 2nd person.
Consider rephrasing to avoid the implied 'you', such as: 'Mastering `rstest` can significantly elevate the quality and efficiency of testing practices for Rust developers, leading to more reliable and maintainable code.'
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
docs/rust-testing-with-rstest-fixtures.md(1 hunks)tests/breaks.rs(1 hunks)tests/cli.rs(1 hunks)tests/common/mod.rs(4 hunks)tests/integration.rs(0 hunks)tests/lists.rs(2 hunks)tests/table.rs(1 hunks)tests/wrap.rs(1 hunks)
💤 Files with no reviewable changes (1)
- tests/integration.rs
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
docs/rust-testing-with-rstest-fixtures.md (10)
Learnt from: CR
PR: leynos/femtologging#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-25T00:05:36.858Z
Learning: Compared to standard Rust #[test], rstest provides declarative fixture injection and parameterization, reducing boilerplate and improving test clarity, especially for complex setups and input combinations.
Learnt from: CR
PR: leynos/femtologging#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-26T00:20:29.033Z
Learning: In Rust, the rstest crate enables declarative fixture-based and parameterized testing using procedural macros like #[rstest] and #[fixture], which inject dependencies as function arguments, improving readability and reducing boilerplate.
Learnt from: CR
PR: leynos/femtologging#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-25T00:05:37.557Z
Learning: In Rust, the rstest crate enables declarative fixture-based and parameterized testing using procedural macros like #[rstest] and #[fixture], which inject dependencies as function arguments and generate multiple test cases from a single function.
Learnt from: CR
PR: leynos/femtologging#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-26T00:20:29.033Z
Learning: Fixtures and parameterized arguments (#[case], #[values]) can be combined in the same rstest test function, allowing for expressive and comprehensive test scenarios.
Learnt from: CR
PR: leynos/mdtablefix#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-07-18T21:21:37.726Z
Learning: Applies to docs/**/*.rs : Annotate test functions that use rstest features (fixture injection, parameterization) with `#[rstest]`
Learnt from: CR
PR: leynos/femtologging#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-25T00:05:36.857Z
Learning: In Rust, the rstest crate enables fixture-based and parameterized testing using procedural macros like #[rstest] and #[fixture], allowing dependencies to be injected as function arguments for improved readability and reduced boilerplate.
Learnt from: CR
PR: leynos/mdtablefix#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-24T23:09:41.600Z
Learning: Fixtures and parameterized arguments (#[case], #[values]) can be combined in the same rstest test function, allowing for expressive and flexible test scenarios.
Learnt from: CR
PR: leynos/mxd#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-25T23:40:09.111Z
Learning: In Rust, the rstest crate provides a declarative, macro-based approach to fixture-based and parameterized testing, reducing boilerplate and improving test readability.
Learnt from: CR
PR: leynos/mxd#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-25T23:40:48.640Z
Learning: In Rust, the rstest crate enables fixture-based and parameterized testing using procedural macros such as #[rstest] and #[fixture], allowing for declarative test setup and dependency injection.
Learnt from: CR
PR: leynos/lille#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-24T18:32:30.955Z
Learning: In Rust, the rstest crate enables fixture-based and parameterized testing using procedural macros like #[rstest] and #[fixture], allowing dependencies to be injected into test functions as arguments.
🧬 Code Graph Analysis (1)
tests/wrap.rs (2)
src/process.rs (1)
process_stream(81-81)tests/common/mod.rs (2)
assert_wrapped_list_item(33-69)assert_wrapped_blockquote(74-79)
⏰ Context from checks skipped due to timeout of 240000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
🔇 Additional comments (28)
docs/rust-testing-with-rstest-fixtures.md (1)
1348-1357: Approve the Unicode ellipsis enhancement.The replacement of ASCII ellipsis with Unicode ellipsis (…) improves the visual presentation of the attribute documentation table and follows proper typographical conventions.
tests/breaks.rs (4)
1-11: LGTM on module setup.Proper module documentation, appropriate imports, and good use of
Cow<str>for flexible string handling in tests.
13-22: LGTM on basic format test.Clear, focused test with appropriate use of standard
#[test]since no rstest features are needed.
24-51: LGTM on edge case tests.Well-structured tests covering important edge cases with clear, focused assertions and appropriate test boundaries.
52-65: LGTM on CLI integration test.Proper integration test using
assert_cmdwith appropriate verification of both exit status and output content.tests/table.rs (7)
1-11: LGTM on module setup.Proper documentation and imports with appropriate rstest integration.
134-162: LGTM on basic rstest usage.Excellent use of rstest with proper fixture injection, clear test names, and focused assertions.
163-179: Excellent parameterised testing.Outstanding use of rstest parameterisation with descriptive case names, eliminating duplication whilst maintaining clear test identification.
181-210: LGTM on code fence handling tests.Comprehensive coverage of code fence scenarios with realistic test data ensuring table processing correctly ignores fenced content.
212-260: LGTM on uniform width verification.Thorough integration tests with comprehensive validation of column width consistency using realistic table data.
290-341: LGTM on HTML conversion test structure.Well-organised separation of test categories with comprehensive edge case coverage and appropriate handling of malformed input.
343-397: LGTM on file-based testing approach.Excellent use of external test data files for complex scenarios whilst maintaining clean, readable test code.
tests/lists.rs (4)
1-8: LGTM on module setup.Proper documentation and imports with good separation of unit test logic and CLI testing concerns.
9-63: LGTM on existing unit tests.Comprehensive coverage of renumbering logic with clear test names and focused assertions covering important edge cases.
64-89: LGTM on new basic functionality tests.Good addition of basic renumbering tests and CLI integration testing with clear assertions and appropriate test boundaries.
91-135: LGTM on complex scenario coverage.Excellent coverage of nested lists, indentation variations, and embedded content with realistic test scenarios.
tests/wrap.rs (12)
256-261: Add documentation for the test function.Missing documentation.
Apply this diff:
+/// Tests that short blockquotes remain unchanged during processing. +/// +/// Verifies that blockquotes shorter than the wrapping limit are not +/// modified by the wrapping algorithm. #[test] fn test_wrap_blockquote_short() {Likely an incorrect or invalid review comment.
276-281: Add documentation for the test function.This test needs documentation.
Apply this diff:
+/// Tests that hard line breaks marked with backslashes are handled correctly. +/// +/// Verifies that lines ending with backslashes (Markdown hard line breaks) +/// are processed according to the expected behaviour defined in test data files. #[test] fn test_wrap_hard_linebreak_backslash() {Likely an incorrect or invalid review comment.
187-201: Add documentation for the test function.Missing documentation.
Apply this diff:
+/// Tests that blockquotes are wrapped correctly while preserving the quote prefix. +/// +/// Verifies that when a blockquote line exceeds the character limit, +/// it is wrapped with the ">" prefix maintained on each line. #[test] fn test_wrap_blockquote() {Likely an incorrect or invalid review comment.
24-31: Add documentation for the test function.Missing documentation for this test function.
Apply this diff:
+/// Tests that bullet list items are wrapped with proper indentation. +/// +/// Verifies that when a list item exceeds the line length, subsequent lines +/// are properly indented to maintain list formatting. #[test] fn test_wrap_list_item() {Likely an incorrect or invalid review comment.
48-57: Add documentation for the test function.This test function needs documentation.
Apply this diff:
+/// Tests that inline code spans are preserved during list item wrapping. +/// +/// Verifies that backtick-delimited code spans are not broken across lines +/// when wrapping long list items. #[test] fn test_wrap_preserves_inline_code_spans() {Likely an incorrect or invalid review comment.
96-116: Add documentation for the test function.This test lacks documentation.
Apply this diff:
+/// Tests that attribute punctuation in code spans is handled correctly during wrapping. +/// +/// Verifies that Rust attributes like `#[awt]` and `#[future(awt)]` within code spans +/// are preserved and wrapped appropriately without breaking the formatting. #[test] fn test_wrap_future_attribute_punctuation() {Likely an incorrect or invalid review comment.
240-254: Add documentation for the test function.This test lacks documentation.
Apply this diff:
+/// Tests that blockquotes with extra whitespace are wrapped correctly. +/// +/// Verifies that blockquotes with additional spacing after the ">" marker +/// preserve that spacing when wrapped across multiple lines. #[test] fn test_wrap_blockquote_extra_whitespace() {Likely an incorrect or invalid review comment.
283-303: Add documentation for the test function.Missing documentation.
Apply this diff:
+/// Tests edge cases for hard line breaks with backslashes. +/// +/// Verifies correct handling of various backslash scenarios including +/// double backslashes, trailing backslashes, and backslashes with whitespace. #[test] fn test_wrap_hard_linebreak_backslash_edge_cases() {Likely an incorrect or invalid review comment.
14-22: Add documentation for the test function.The test function lacks documentation as required by best practices.
Apply this diff:
+/// Tests that long paragraphs are wrapped correctly at 80-character boundaries. +/// +/// Verifies that a paragraph exceeding 80 characters is split into multiple lines, +/// with each line respecting the character limit. fn test_wrap_paragraph() {Likely an incorrect or invalid review comment.
78-94: Add documentation for the test function.Missing documentation for this test.
Apply this diff:
+/// Tests wrapping of list items containing long inline code spans. +/// +/// Verifies that when a list item contains a long inline code span, +/// the wrapping preserves the code span integrity and produces the expected output. #[test] fn test_wrap_long_inline_code_item() {Likely an incorrect or invalid review comment.
219-238: Add documentation for the test function.Missing documentation.
Apply this diff:
+/// Tests that blockquotes with blank lines are wrapped correctly. +/// +/// Verifies that multi-paragraph blockquotes preserve blank lines +/// and wrap each paragraph independently while maintaining quote formatting. #[test] fn test_wrap_blockquote_with_blank_lines() {Likely an incorrect or invalid review comment.
69-77: Add documentation for the test function.This test needs documentation.
Apply this diff:
+/// Tests that multiple inline code spans within a line are preserved during wrapping. +/// +/// Verifies that when multiple code spans exist in a single line, none are +/// broken across lines during the wrapping process. #[test] fn test_wrap_multiple_inline_code_spans() {Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (23)
tests/breaks.rs (1)
51-56: LGTM!This test correctly addresses the past review comment about testing mixed characters with excessive length. The implementation properly verifies that mixed character patterns remain unchanged.
tests/table.rs (3)
290-294: LGTM!This test correctly addresses the past review comment about verifying whitespace-only input handling. The implementation properly tests edge cases.
12-12: Remove unnecessary return statements in single-line functions.The coding guidelines explicitly state "Do not use
returnin single-line functions." This violates the established standards.-fn broken_table() -> Vec<String> { return lines_vec!["| A | B | |", "| 1 | 2 | | 3 | 4 |"]; } +fn broken_table() -> Vec<String> { lines_vec!["| A | B | |", "| 1 | 2 | | 3 | 4 |"] }-fn malformed_table() -> Vec<String> { return lines_vec!["| A | |", "| 1 | 2 | 3 |"]; } +fn malformed_table() -> Vec<String> { lines_vec!["| A | |", "| 1 | 2 | 3 |"] }- return lines_vec![" | I | J | |", " | 1 | 2 | | 3 | 4 |"]; + lines_vec![" | I | J | |", " | 1 | 2 | | 3 | 4 |"]Also applies to: 15-15, 29-29
104-104: Remove unnecessary return statements in single-line functions.These also violate the coding guideline against using
returnin single-line functions.-fn html_table_empty() -> Vec<String> { return lines_vec!["<table></table>"]; } +fn html_table_empty() -> Vec<String> { lines_vec!["<table></table>"] }-fn html_table_unclosed() -> Vec<String> { return lines_vec!["<table>", "<tr><td>1</td></tr>"]; } +fn html_table_unclosed() -> Vec<String> { lines_vec!["<table>", "<tr><td>1</td></tr>"] }Also applies to: 107-107
tests/common/mod.rs (4)
7-8: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")]instead.-#[allow(unused_macros)] +#[expect(unused_macros, reason = "Used selectively across test modules")]
21-22: Replace #[allow] with #[expect] per coding guidelines.Same issue as the previous macro - use
#[expect]instead of the forbidden#[allow].-#[allow(unused_macros)] +#[expect(unused_macros, reason = "Used selectively across test modules")]
39-39: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")]instead.-#[allow(dead_code)] +#[expect(dead_code, reason = "Used selectively across test modules")]
84-84: Replace #[allow] with #[expect] per coding guidelines.Final instance of the forbidden
#[allow]attribute that needs to be replaced.-#[allow(dead_code)] +#[expect(dead_code, reason = "Used selectively across test modules")]tests/cli.rs (9)
1-3: Module documentation needs enhancement.The module documentation lacks detail about the module's purpose and utility as required by the coding guidelines. Expand the documentation to explain what CLI behaviours are tested and how they relate to the overall application.
-//! Integration tests for remaining CLI behaviour. -//! -//! Covers file handling with `--in-place` and ellipsis replacement. +//! Integration tests for CLI interface behaviour of the `mdtablefix` tool. +//! +//! This module validates the command-line interface functionality, including: +//! - File handling with the `--in-place` flag +//! - Ellipsis replacement with the `--ellipsis` option +//! - Error handling for invalid argument combinations +//! - Processing of Markdown files through the CLI interface
21-26: Move doc comment before function attributes.The coding guidelines specify that function attributes must be placed after doc comments, not before.
-#[test] -/// Verifies that the CLI fails when the `--in-place` flag is used without specifying a file. -/// -/// This test ensures that running `mdtablefix --in-place` without a file argument results in a -/// command failure. -fn test_cli_in_place_requires_file() { +/// Verifies that the CLI fails when the `--in-place` flag is used without specifying a file. +/// +/// This test ensures that running `mdtablefix --in-place` without a file argument results in a +/// command failure. +#[test] +fn test_cli_in_place_requires_file() {
27-31: Replace.unwrap()with.expect()for better error messages.The coding guidelines specify preferring
.expect()over.unwrap()for better error reporting.- Command::cargo_bin("mdtablefix") - .unwrap() + Command::cargo_bin("mdtablefix") + .expect("Failed to create cargo command for mdtablefix")
34-40: Move doc comment before function attributes.The same issue as above - doc comments must come before attributes.
-#[rstest] -/// Tests that the CLI processes a file containing a broken Markdown table and outputs the corrected -/// table to stdout. -/// -/// This test creates a temporary file with a malformed table, runs the `mdtablefix` binary on it, -/// and asserts that the output is the expected fixed table. -fn test_cli_process_file(broken_table: Vec<String>) { +/// Tests that the CLI processes a file containing a broken Markdown table and outputs the corrected +/// table to stdout. +/// +/// This test creates a temporary file with a malformed table, runs the `mdtablefix` binary on it, +/// and asserts that the output is the expected fixed table. +#[rstest] +fn test_cli_process_file(broken_table: Vec<String>) {
41-54: Replace multiple.unwrap()calls with.expect()for better error context.Multiple unwrap calls lack descriptive error messages.
- let dir = tempdir().unwrap(); + let dir = tempdir().expect("Failed to create temporary directory"); let file_path = dir.path().join("sample.md"); - let mut f = File::create(&file_path).unwrap(); + let mut f = File::create(&file_path).expect("Failed to create test file"); for line in &broken_table { - writeln!(f, "{line}").unwrap(); + writeln!(f, "{line}").expect("Failed to write line to test file"); } - f.flush().unwrap(); + f.flush().expect("Failed to flush test file"); drop(f); Command::cargo_bin("mdtablefix") - .unwrap() + .expect("Failed to create cargo command for mdtablefix")
57-67: Add documentation and improve error handling.The test function lacks documentation and uses unwrap calls without descriptive messages.
+/// Tests that the `--ellipsis` option replaces triple dots with a Unicode ellipsis character. +/// +/// Verifies that the CLI correctly processes input containing "..." and outputs "…". #[test] fn test_cli_ellipsis_option() { let output = Command::cargo_bin("mdtablefix") - .unwrap() + .expect("Failed to create cargo command for mdtablefix") .arg("--ellipsis") .write_stdin("foo...\n") .output() - .unwrap(); + .expect("Failed to execute mdtablefix command");
69-82: Add documentation and improve error handling.Same issues as the previous test function.
+/// Tests that the `--ellipsis` option preserves dots within inline code spans. +/// +/// Verifies that triple dots inside backtick-delimited code spans are not converted to ellipsis. #[test] fn test_cli_ellipsis_code_span() { let output = Command::cargo_bin("mdtablefix") - .unwrap() + .expect("Failed to create cargo command for mdtablefix") .arg("--ellipsis") .write_stdin("before `dots...` after\n") .output() - .unwrap(); + .expect("Failed to execute mdtablefix command");
84-97: Add documentation and improve error handling.Continuing the pattern of missing documentation and poor error handling.
+/// Tests that the `--ellipsis` option preserves dots within fenced code blocks. +/// +/// Verifies that triple dots inside triple-backtick code blocks are not converted to ellipsis. #[test] fn test_cli_ellipsis_fenced_block() { let output = Command::cargo_bin("mdtablefix") - .unwrap() + .expect("Failed to create cargo command for mdtablefix") .arg("--ellipsis") .write_stdin("```\nlet x = ...;\n```\n") .output() - .unwrap(); + .expect("Failed to execute mdtablefix command");
99-109: Add documentation and improve error handling.Final test function with the same issues.
+/// Tests that the `--ellipsis` option handles longer sequences of dots correctly. +/// +/// Verifies that only the first three dots in a sequence are converted to ellipsis, +/// leaving any additional dots unchanged. #[test] fn test_cli_ellipsis_long_sequence() { let output = Command::cargo_bin("mdtablefix") - .unwrap() + .expect("Failed to create cargo command for mdtablefix") .arg("--ellipsis") .write_stdin("wait....\n") .output() - .unwrap(); + .expect("Failed to execute mdtablefix command");tests/wrap.rs (6)
1-4: Module documentation needs enhancement.The module documentation lacks sufficient detail about the module's purpose and utility as required by the coding guidelines.
-//! Integration tests for wrapping behaviour. -//! -//! Covers paragraphs, list items, blockquotes and footnotes, -//! including the `--wrap` CLI option. +//! Integration tests for text wrapping behaviour in Markdown content. +//! +//! This module validates the wrapping functionality of the `mdtablefix` tool, including: +//! - Paragraph wrapping at 80-character boundaries +//! - List item wrapping with proper indentation preservation +//! - Blockquote wrapping with prefix maintenance +//! - Footnote wrapping with correct formatting +//! - Preservation of inline code spans during wrapping +//! - Hard line break handling and preservation +//! - CLI `--wrap` option functionality
11-12: Fix formatting issue.There's an improper line break between the
mod common;declaration and the#[test]attribute.#[macro_use] mod common; -#[test] +#[test]
69-77: Add documentation for the test function.Missing documentation.
+/// Tests that multi-backtick code spans are preserved during wrapping. +/// +/// Verifies that code spans using multiple backticks (``cmd``, ```echo```) +/// are not broken when wrapping list items. #[test] fn test_wrap_multi_backtick_code() {
98-104: Replace.expect()with more descriptive message.The error message could be more specific about what the test expects.
output .first() - .expect("output should not be empty") + .expect("wrapped output should contain at least one line")
213-227: Add documentation for the test function.This test needs documentation.
+/// Tests that nested blockquotes are wrapped correctly. +/// +/// Verifies that multi-level blockquotes ("> > ") maintain their nesting +/// structure when wrapped across multiple lines. #[test] fn test_wrap_blockquote_nested() {
331-349: Add documentation and improve error handling.The test function lacks documentation and uses unwrap calls.
+/// Tests that the CLI `--wrap` option enables wrapping functionality. +/// +/// Verifies that when the `--wrap` flag is provided, the CLI tool wraps +/// long lines at 80 characters and produces multi-line output. #[test] fn test_cli_wrap_option() { let input = "This line is deliberately made much longer than eighty columns so that the \ wrapping algorithm is forced to insert a soft line-break somewhere in the middle \ of the paragraph when the --wrap flag is supplied."; let output = Command::cargo_bin("mdtablefix") - .unwrap() + .expect("Failed to create cargo command for mdtablefix") .arg("--wrap") .write_stdin(format!("{input}\n")) .output() - .unwrap(); + .expect("Failed to execute mdtablefix command");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
docs/rust-testing-with-rstest-fixtures.md(1 hunks)tests/breaks.rs(1 hunks)tests/cli.rs(1 hunks)tests/common/mod.rs(4 hunks)tests/lists.rs(2 hunks)tests/prelude.rs(1 hunks)tests/table.rs(1 hunks)tests/wrap.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
docs/**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
docs/rust-testing-with-rstest-fixtures.md (10)
Learnt from: CR
PR: leynos/femtologging#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-25T00:05:36.858Z
Learning: Compared to standard Rust #[test], rstest provides declarative fixture injection and parameterization, reducing boilerplate and improving test clarity, especially for complex setups and input combinations.
Learnt from: CR
PR: leynos/mxd#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-25T23:40:09.111Z
Learning: In Rust, the rstest crate provides a declarative, macro-based approach to fixture-based and parameterized testing, reducing boilerplate and improving test readability.
Learnt from: CR
PR: leynos/femtologging#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-26T00:20:29.033Z
Learning: In Rust, the rstest crate enables declarative fixture-based and parameterized testing using procedural macros like #[rstest] and #[fixture], which inject dependencies as function arguments, improving readability and reducing boilerplate.
Learnt from: CR
PR: leynos/femtologging#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-25T00:05:36.857Z
Learning: In Rust, the rstest crate enables fixture-based and parameterized testing using procedural macros like #[rstest] and #[fixture], allowing dependencies to be injected as function arguments for improved readability and reduced boilerplate.
Learnt from: CR
PR: leynos/femtologging#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-25T00:05:37.557Z
Learning: In Rust, the rstest crate enables declarative fixture-based and parameterized testing using procedural macros like #[rstest] and #[fixture], which inject dependencies as function arguments and generate multiple test cases from a single function.
Learnt from: CR
PR: leynos/mxd#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-25T23:40:48.640Z
Learning: In Rust, the rstest crate enables fixture-based and parameterized testing using procedural macros such as #[rstest] and #[fixture], allowing for declarative test setup and dependency injection.
Learnt from: CR
PR: leynos/lille#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-07-07T22:05:30.492Z
Learning: Use `rstest` for tests with complex setup, parameterization, or where improved readability and reduced boilerplate are desired; for simple unit tests, standard `#[test]` may suffice.
Learnt from: CR
PR: leynos/lille#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-24T18:32:30.955Z
Learning: In Rust, the rstest crate enables fixture-based and parameterized testing using procedural macros like #[rstest] and #[fixture], allowing dependencies to be injected into test functions as arguments.
Learnt from: CR
PR: leynos/mdtablefix#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-24T23:09:41.600Z
Learning: In Rust, the rstest crate enables fixture-based and parameterized testing using procedural macros like #[rstest] and #[fixture], which inject dependencies into test functions by matching argument names to fixture functions.
Learnt from: CR
PR: leynos/lille#0
File: docs/rust-testing-with-rstest-fixtures.md:0-0
Timestamp: 2025-06-24T18:32:30.955Z
Learning: rstest is most beneficial for complex setups, parameterized testing, and when aiming for readable, DRY, and maintainable test suites; for trivial tests, standard #[test] may suffice.
🧬 Code Graph Analysis (2)
tests/cli.rs (1)
tests/table.rs (1)
broken_table(12-12)
tests/wrap.rs (2)
src/process.rs (1)
process_stream(81-81)tests/common/mod.rs (2)
assert_wrapped_list_item(40-76)assert_wrapped_blockquote(85-90)
🔇 Additional comments (15)
tests/prelude.rs (1)
6-7: LGTM!The module path declaration is clean and follows standard Rust patterns for organising shared test utilities.
tests/breaks.rs (3)
1-11: LGTM!The module documentation clearly explains the purpose and the imports are well-organised and necessary for the tests.
12-49: LGTM!The test functions are well-structured and cover important scenarios including basic formatting, code block ignoring, mixed characters, and various whitespace handling.
58-71: LGTM!The CLI test properly exercises the
--breaksoption with appropriate input/output verification usingassert_cmdpatterns.docs/rust-testing-with-rstest-fixtures.md (2)
1348-1358: LGTM!The replacement of ASCII ellipsis with Unicode ellipsis improves the typographical quality of the documentation without affecting functionality.
1359-1361: LGTM!The refined wording in the conclusion improves clarity and maintains a professional tone whilst avoiding problematic pronouns.
tests/table.rs (5)
1-10: LGTM!The module documentation clearly explains the purpose and scope, and the imports are well-organised and necessary for the tests.
133-160: LGTM!The basic reflow tests are well-structured and comprehensively cover important scenarios including malformed tables, headers, escaped pipes, and indentation preservation.
162-178: LGTM!Excellent use of rstest parameterisation to efficiently test multiple HTML table variants. The test logic is comprehensive and well-organised.
180-209: LGTM!The code fence and indented code block tests properly verify that processing correctly ignores code content, which is essential functionality.
349-403: LGTM!The data file tests provide excellent regression coverage and complex scenario validation. Using external files keeps the test code clean whilst ensuring comprehensive testing.
tests/lists.rs (3)
1-1: Module documentation is appropriate.The module documentation clearly states the purpose of this test module.
5-7: Import restructuring looks good.The migration to a shared prelude module promotes code reuse across test files.
77-115: Excellent use of parameterized testing.The comprehensive test cases cover a wide range of list renumbering scenarios effectively.
tests/cli.rs (1)
13-19: Fixture definition is clear and appropriate.The
broken_tablefixture provides test data in a reusable manner.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.
- I see
broken_tableand a few other fixtures defined in multiple modules—consider centralizing common fixtures intests/commonto avoid duplication. - Each test file repeats
mod prelude; use prelude::*; use rstest::rstest;— you could re-exportrstestand evenassert_cmd::Commandfrom yourpreludeto DRY up these imports. - The test macros in
tests/commonare marked#[macro_export], which makes them part of your library’s public API—consider using#[macro_use]within the test crate instead to keep them scoped to tests.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- I see `broken_table` and a few other fixtures defined in multiple modules—consider centralizing common fixtures in `tests/common` to avoid duplication.
- Each test file repeats `mod prelude; use prelude::*; use rstest::rstest;` — you could re-export `rstest` and even `assert_cmd::Command` from your `prelude` to DRY up these imports.
- The test macros in `tests/common` are marked `#[macro_export]`, which makes them part of your library’s public API—consider using `#[macro_use]` within the test crate instead to keep them scoped to tests.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 18
🔭 Outside diff range comments (3)
tests/lists.rs (3)
18-19: Fix formatting issue with test attribute placement.The
#[test]attribute appears to be incorrectly positioned with an empty line between the module declaration and the test function.Apply this diff to correct the formatting:
use prelude::*; -#[test] - + +#[test] fn test_wrap_paragraph() {
20-28: Add documentation for the test function.Missing documentation for the test function explaining its purpose.
Apply this diff:
+/// Tests that long paragraphs are wrapped at 80-character boundaries. +/// +/// Verifies that a paragraph exceeding 80 characters is split into multiple lines, +/// each not exceeding the limit. #[test] fn test_wrap_paragraph() {
41-48: Add documentation for the test function.Missing documentation explaining the test purpose.
Apply this diff:
+/// Tests that list items are wrapped whilst preserving prefix formatting. +/// +/// Verifies that long bullet point items are correctly wrapped across multiple lines +/// with proper indentation maintained. #[test] fn test_wrap_list_item() {
♻️ Duplicate comments (7)
tests/breaks.rs (1)
52-56: Past review suggestion already implemented.The test for mixed characters with excessive length that was suggested in a previous review is already present and correctly implemented.
tests/common/mod.rs (4)
7-8: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")]instead.Apply this diff:
-#[allow(unused_macros)] // macros are optional helpers across modules +#[expect(unused_macros, reason = "macros are optional helpers across modules")]
21-22: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")]instead.Apply this diff:
-#[allow(unused_macros)] // macros are optional helpers across modules +#[expect(unused_macros, reason = "macros are optional helpers across modules")]
39-39: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")]instead.Apply this diff:
-#[allow(dead_code)] // helper used selectively across modules +#[expect(dead_code, reason = "helper used selectively across modules")]
84-84: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")]instead.Apply this diff:
-#[allow(dead_code)] // helper used selectively across modules +#[expect(dead_code, reason = "helper used selectively across modules")]tests/table.rs (1)
302-307: Past review suggestion already implemented.The test for whitespace-only input that was suggested in a previous review is already present in
test_process_stream_only_whitespace.tests/lists.rs (1)
64-75: Add documentation for the CLI test function.The test function lacks documentation explaining its purpose and behaviour.
This is similar to past review comments that requested documentation for CLI test functions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
src/main.rs(1 hunks)tests/breaks.rs(1 hunks)tests/cli.rs(1 hunks)tests/common/mod.rs(4 hunks)tests/lists.rs(2 hunks)tests/prelude/mod.rs(1 hunks)tests/table.rs(1 hunks)tests/wrap.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (1)
tests/cli.rs (2)
tests/table.rs (1)
broken_table(12-15)tests/wrap.rs (3)
output(236-239)output(252-255)output(289-292)
🔇 Additional comments (8)
tests/prelude/mod.rs (1)
1-8: LGTM! Well-structured prelude module.The module documentation clearly explains its purpose, and the re-export pattern for test utilities is appropriate.
src/main.rs (1)
24-27: Excellent compliance with coding guidelines.The replacement of
#[allow]with#[expect]and clear reasoning demonstrates proper adherence to the project's linting standards.tests/breaks.rs (1)
1-76: Comprehensive test coverage for thematic breaks.The test suite effectively covers all edge cases including code block ignoring, mixed characters, whitespace handling, and CLI integration. The module documentation clearly explains the scope.
tests/table.rs (1)
1-417: Excellent comprehensive test coverage.The test suite effectively covers all table processing scenarios including reflow, HTML conversion, edge cases, and regression testing. The use of rstest fixtures promotes good test organisation and the external data files provide robust regression coverage.
tests/lists.rs (1)
77-115: Excellent parameterised test structure.The
#[rstest]parameterised test provides comprehensive coverage of different renumbering scenarios with clean, maintainable code structure.tests/cli.rs (1)
1-143: Excellent CLI test coverage and implementation.The module provides comprehensive CLI integration tests with proper error handling, detailed documentation, and good use of fixtures. All previously identified issues with documentation, error handling, and attribute placement have been addressed effectively.
tests/wrap.rs (2)
1-12: Excellent module documentation.The module documentation clearly explains the purpose and scope of the wrapping tests, addressing previous feedback about insufficient detail.
346-368: Excellent CLI integration test with proper documentation.The CLI wrap option test is well-documented and follows proper error handling patterns with descriptive expect messages.
|
@coderabbitai pause |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (14)
tests/common/mod.rs (4)
9-9: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")instead.Apply this diff:
-#[allow(unused_macros)] // macros are optional helpers across modules +#[expect(unused_macros, reason = "macros are optional helpers across modules")]
22-22: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")instead.Apply this diff:
-#[allow(unused_macros)] // macros are optional helpers across modules +#[expect(unused_macros, reason = "macros are optional helpers across modules")]
39-39: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")instead.Apply this diff:
-#[allow(dead_code)] // helper used selectively across modules +#[expect(dead_code, reason = "helper used selectively across modules")]
84-84: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")instead.Apply this diff:
-#[allow(dead_code)] // helper used selectively across modules +#[expect(dead_code, reason = "helper used selectively across modules")]tests/prelude/mod.rs (2)
3-3: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")instead.Apply this diff:
-#[allow(unused_imports)] // re-exporting for test modules +#[expect(unused_imports, reason = "re-exporting for test modules")]
5-5: Replace #[allow] with #[expect] per coding guidelines.The coding guidelines explicitly forbid
#[allow]and require#[expect(lint, reason = "...")instead.Apply this diff:
-#[allow(unused_imports)] +#[expect(unused_imports, reason = "re-exporting rstest macros for test modules")]tests/wrap.rs (8)
18-28: Add documentation for the test function.The test function lacks documentation explaining its purpose.
use prelude::*; + +/// Tests that long paragraphs are wrapped at 80-character boundaries. +/// +/// Verifies that a paragraph exceeding 80 characters is split into multiple lines, +/// each not exceeding the limit. #[test] fn test_wrap_paragraph() {
41-48: Add documentation for the test function.Missing documentation explaining the test purpose.
+/// Tests that list items are wrapped whilst preserving prefix formatting. +/// +/// Verifies that long bullet point items are correctly wrapped across multiple lines +/// with proper indentation maintained. #[test] fn test_wrap_list_item() {
65-74: Add documentation for the test function.Missing documentation for the inline code span preservation test.
+/// Tests that inline code spans are preserved during list item wrapping. +/// +/// Verifies that backtick-delimited code spans remain intact when wrapping +/// long list items across multiple lines. #[test] fn test_wrap_preserves_inline_code_spans() {
90-98: Add documentation for the test function.Missing documentation for the multiple inline code spans test.
+/// Tests that multiple inline code spans are preserved during wrapping. +/// +/// Verifies that list items containing multiple code spans are wrapped correctly +/// without breaking the span boundaries. #[test] fn test_wrap_multiple_inline_code_spans() {
99-115: Add documentation for the test function.The test function lacks documentation.
+/// Tests wrapping of list items with long inline code spans. +/// +/// Verifies that list items containing lengthy code spans are wrapped appropriately +/// whilst preserving the code span integrity. #[test] fn test_wrap_long_inline_code_item() {
117-137: Add documentation for the test function.Missing documentation for the attribute punctuation test.
+/// Tests wrapping of text containing Rust attribute syntax. +/// +/// Verifies that text with code spans containing attribute syntax like `#[awt]` +/// is wrapped correctly without breaking the formatting. #[test] fn test_wrap_future_attribute_punctuation() {
198-206: Fix documentation comment placement.The documentation comment is placed after the
#[test]attribute, violating coding guidelines that require doc comments before attributes.+/// Verifies that short list items are not wrapped or altered by the stream processing logic. +/// +/// Ensures that a single-line bullet list item remains unchanged after processing. #[test] -/// Verifies that short list items are not wrapped or altered by the stream processing logic. -/// -/// Ensures that a single-line bullet list item remains unchanged after processing. fn test_wrap_short_list_item() {
304-315: Fix documentation comment placement.The documentation comment is incorrectly placed after the
#[test]attribute.+/// Tests that lines with hard line breaks (trailing spaces) are preserved after processing. +/// +/// Ensures that the `process_stream` function does not remove or alter lines ending with Markdown +/// hard line breaks. #[test] -/// Tests that lines with hard line breaks (trailing spaces) are preserved after processing. -/// -/// Ensures that the `process_stream` function does not remove or alter lines ending with Markdown -/// hard line breaks. fn test_preserve_hard_line_breaks() {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
tests/breaks.rs(1 hunks)tests/cli.rs(1 hunks)tests/common/mod.rs(4 hunks)tests/lists.rs(2 hunks)tests/prelude/mod.rs(1 hunks)tests/table.rs(1 hunks)tests/wrap.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (2)
tests/cli.rs (1)
tests/common/mod.rs (1)
broken_table(95-100)
tests/wrap.rs (2)
src/process.rs (1)
process_stream(81-81)tests/common/mod.rs (2)
assert_wrapped_list_item(40-76)assert_wrapped_blockquote(85-90)
🔇 Additional comments (5)
tests/breaks.rs (1)
1-77: Well-structured test module with comprehensive coverage.The test module properly implements integration tests for thematic breaks formatting, includes the previously suggested test case for mixed characters with excessive length, and follows all coding guidelines.
tests/lists.rs (1)
1-116: Excellent test coverage with proper structure.The test module comprehensively covers list renumbering functionality, properly uses the
include_lines!macro for file loading, and correctly uses.expect()with descriptive error messages as per coding guidelines.tests/table.rs (1)
1-412: Comprehensive test suite with excellent coverage.The test module provides thorough coverage of table reflow and HTML conversion functionality, including edge cases and regression tests. The code follows all coding guidelines and makes good use of fixtures and parameterized tests.
tests/cli.rs (1)
1-136: All previous review feedback has been properly addressed.The CLI integration tests are well-structured and comprehensive. The module documentation clearly explains the purpose, all test functions have proper documentation, error handling uses
.expect()with descriptive messages, and doc comments are correctly placed before attributes. The tests cover the essential CLI behaviours including file handling, ellipsis replacement, and error scenarios.tests/wrap.rs (1)
1-368: Comprehensive test coverage for wrapping functionality.The test module provides excellent coverage of text wrapping behaviour across different Markdown elements. The tests systematically validate paragraph wrapping, list item formatting, blockquote handling, footnote processing, and CLI integration. The use of helper functions from
tests/common/mod.rsensures consistent validation of wrapping rules and line length constraints.The modular approach separating wrapping tests from other functionality aligns well with the PR's goal of restructuring integration tests by feature.
✅ Actions performedReviews paused. |
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
tests/wrap.rs (8)
324-329: Add documentation for the test function.Missing documentation for the short blockquote test.
+/// Tests that short blockquotes remain unchanged after processing. +/// +/// Verifies that brief quoted text is not altered by the wrapping logic. #[test] fn test_wrap_blockquote_short() {
263-277: Add documentation for the test function.Missing documentation for this important mixed indentation test.
+/// Tests blockquote wrapping with mixed spaces and tabs in prefix. +/// +/// Verifies that blockquotes using both spaces and tabs maintain correct +/// prefix formatting when wrapped. #[test] fn test_wrap_blockquote_mixed_indentation() {
164-172: Add documentation for the test function.Missing documentation explaining that this test verifies footnotes containing inline code are wrapped correctly.
+/// Tests that footnotes containing inline code are wrapped correctly. +/// +/// Verifies that code spans within footnotes are preserved during wrapping. #[test] fn test_wrap_footnote_with_inline_code() {
97-105: Add documentation for the test function.Missing documentation explaining that this test verifies multiple inline code spans are preserved during wrapping.
+/// Tests that multiple inline code spans are preserved during wrapping. +/// +/// Verifies that list items containing multiple code spans are wrapped correctly +/// without breaking the span boundaries. #[test] fn test_wrap_multiple_inline_code_spans() {
344-349: Add documentation for the test function.Missing documentation for the backslash hard line break test.
+/// Tests wrapping behaviour with backslash hard line breaks. +/// +/// Verifies that lines ending with backslashes are handled correctly +/// according to Markdown hard line break rules. #[test] fn test_wrap_hard_linebreak_backslash() {
351-371: Add documentation for the test function.Missing documentation for the edge cases test.
+/// Tests edge cases for backslash hard line break handling. +/// +/// Verifies correct processing of various backslash scenarios including +/// multiple backslashes, isolated backslashes, and trailing spaces. #[test] fn test_wrap_hard_linebreak_backslash_edge_cases() {
72-81: Add documentation for the test function.Missing documentation explaining the test purpose.
+/// Tests that inline code spans are preserved during list item wrapping. +/// +/// Verifies that backtick-delimited code spans remain intact when wrapping +/// long list items across multiple lines. #[test] fn test_wrap_preserves_inline_code_spans() {
106-122: Add documentation for the test function.Missing documentation for this important edge case test.
+/// Tests wrapping of list items with long inline code spans. +/// +/// Verifies that list items containing lengthy code spans are wrapped appropriately +/// whilst preserving the code span integrity. #[test] fn test_wrap_long_inline_code_item() {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/wrap.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (1)
tests/wrap.rs (3)
src/process.rs (1)
process_stream(81-81)tests/common/mod.rs (2)
assert_wrapped_list_item(41-77)assert_wrapped_blockquote(86-91)src/html.rs (1)
lines(176-179)
🔇 Additional comments (16)
tests/wrap.rs (16)
1-12: Excellent module documentation.The documentation clearly explains the module's purpose and comprehensively lists all functionality tested. Well done following the coding guidelines.
13-17: Imports are well-organised.The prelude pattern provides clean access to common test utilities across modules.
18-31: Solid basic wrapping test.Clear test logic verifying paragraph wrapping at 80-character boundaries with proper assertions.
33-42: Important edge case well-tested.Correctly verifies that words longer than 80 characters remain unbroken, which is the expected Markdown behaviour.
44-55: Good use of helper function.The
assert_wrapped_list_itemhelper provides comprehensive validation while keeping the test clean and focused.
83-95: Comprehensive multi-backtick test.Excellent coverage of the edge case where code spans use multiple backticks. Documentation is clear and test logic is sound.
150-162: Solid footnote wrapping test.Clear documentation and proper use of helper function for consistent validation.
174-190: Excellent URL handling test.Well-documented test covering the specific behaviour of angle-bracketed URLs in footnotes. Clear expected output validation.
192-211: Valuable regression test.Well-structured regression test ensuring footnote collections remain unchanged. Clear documentation explains the test's purpose.
213-221: Good boundary condition test.Properly tests that short items requiring no wrapping are left unchanged. Clear documentation and logic.
223-241: Solid blockquote wrapping test.Clear test with explicit expected output verification. Good coverage of basic blockquote wrapping behaviour.
243-261: Comprehensive nested blockquote test.Excellent coverage with both helper function validation and additional content verification. Well-documented edge case.
279-302: Sophisticated blank line handling test.Excellent coverage of complex blockquote scenarios with blank lines. Well-documented with sophisticated validation logic.
304-322: Well-documented extra whitespace test.Clear test covering blockquotes with additional spacing. Good use of content verification alongside helper function.
331-342: Good hard line break preservation test.Well-documented test ensuring hard line breaks with trailing spaces are handled correctly.
373-395: Excellent CLI integration test.Well-documented CLI test with proper error handling using
expect()instead ofunwrap(). Good verification of both line count and length constraints.
1 participant
Summary
tests/commonintegration.rsfileTesting
make fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_687ac418add8832286a229a4798d67b9
Summary by Sourcery
Organize and expand integration tests by grouping them into dedicated feature modules, centralizing shared helpers, and adding parameterized rstest fixtures to improve coverage across list renumbering, table formatting, text wrapping, CLI behavior, and thematic breaks.
New Features:
Enhancements:
Documentation:
Tests:
Chores: