Conversation
Summary by CodeRabbit
WalkthroughRestructure and modularise the integration tests for table reflow, HTML table conversion, and Markdown wrapping. Remove the monolithic Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant TableModule
participant WrapModule
TestRunner->>TableModule: Run reflow, HTML conversion, regression, uniformity tests
TestRunner->>WrapModule: Run paragraphs, lists, blockquotes, footnotes, links, CLI tests
TableModule-->>TestRunner: Return test results
WrapModule-->>TestRunner: Return test results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
✨ 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 (
|
Reviewer's GuideThis PR refactors the existing monolithic wrap.rs and table.rs integration tests by splitting them into structured, feature‐specific modules under tests/wrap/ and tests/table/, ensuring each file stays within the 400‐line limit and introducing rstest fixtures and parameterized cases for table scenarios. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `tests/wrap/footnotes.rs:5` </location>
<code_context>
- assert_eq!(process_stream(&lines), lines);
-}
-
-#[test]
-fn test_uniform_example_one() {
- let input = lines_vec![
</code_context>
<issue_to_address>
Consider adding a test for footnotes with multiline content and blank lines.
Adding such a test will help verify that footnotes with blank lines or multiple paragraphs are handled correctly by the wrapping logic.
</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: 12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (16)
tests/table.rs(0 hunks)tests/table/convert_html.rs(1 hunks)tests/table/mod.rs(1 hunks)tests/table/process_stream_tests.rs(1 hunks)tests/table/reflow.rs(1 hunks)tests/table/regressions.rs(1 hunks)tests/table/uniform.rs(1 hunks)tests/wrap.rs(0 hunks)tests/wrap/blockquotes.rs(1 hunks)tests/wrap/cli.rs(1 hunks)tests/wrap/footnotes.rs(1 hunks)tests/wrap/hard_line_breaks.rs(1 hunks)tests/wrap/links.rs(1 hunks)tests/wrap/lists.rs(1 hunks)tests/wrap/mod.rs(1 hunks)tests/wrap/paragraphs.rs(1 hunks)
💤 Files with no reviewable changes (2)
- tests/table.rs
- tests/wrap.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider usingArcto reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallow.
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Files:
tests/wrap/paragraphs.rstests/table/regressions.rstests/wrap/mod.rstests/table/reflow.rstests/wrap/hard_line_breaks.rstests/table/uniform.rstests/table/process_stream_tests.rstests/wrap/footnotes.rstests/wrap/blockquotes.rstests/wrap/lists.rstests/table/mod.rstests/wrap/cli.rstests/wrap/links.rstests/table/convert_html.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/wrap/paragraphs.rstests/table/regressions.rstests/wrap/mod.rstests/table/reflow.rstests/wrap/hard_line_breaks.rstests/table/uniform.rstests/table/process_stream_tests.rstests/wrap/footnotes.rstests/wrap/blockquotes.rstests/wrap/lists.rstests/table/mod.rstests/wrap/cli.rstests/wrap/links.rstests/table/convert_html.rs
🧬 Code Graph Analysis (8)
tests/wrap/mod.rs (1)
src/process.rs (1)
process_stream(206-214)
tests/table/reflow.rs (3)
tests/common/mod.rs (1)
broken_table(97-102)src/table.rs (1)
reflow_table(179-198)tests/table/mod.rs (4)
malformed_table(12-15)header_table(18-20)escaped_pipe_table(23-25)indented_table(28-31)
tests/wrap/hard_line_breaks.rs (1)
src/process.rs (1)
process_stream(206-214)
tests/wrap/footnotes.rs (2)
src/process.rs (1)
process_stream(206-214)tests/common/mod.rs (1)
assert_wrapped_list_item(42-78)
tests/wrap/lists.rs (2)
src/process.rs (1)
process_stream(206-214)tests/common/mod.rs (1)
assert_wrapped_list_item(42-78)
tests/table/mod.rs (3)
src/html.rs (2)
convert_html_tables(302-345)lines(179-182)src/process.rs (1)
process_stream(206-214)src/table.rs (1)
reflow_table(179-198)
tests/wrap/links.rs (2)
src/process.rs (1)
process_stream(206-214)tests/wrap.rs (3)
test_wrap_paragraph_with_link(430-444)test_wrap_link_with_trailing_punctuation(481-488)test_wrap_paragraph_with_image_link(448-460)
tests/table/convert_html.rs (3)
tests/table/mod.rs (10)
html_table(34-41)html_table_with_attrs(44-51)html_table_uppercase(116-123)html_table_with_colspan(54-61)html_table_inconsistent_first_row(94-101)html_table_no_header(64-71)html_table_empty_row(74-81)html_table_whitespace_header(84-91)html_table_empty(104-107)html_table_unclosed(110-113)src/html.rs (1)
convert_html_tables(302-345)tests/table.rs (2)
test_convert_html_table_no_header(326-329)test_convert_html_table_standard(311-313)
🔇 Additional comments (40)
tests/wrap/cli.rs (1)
5-20: LGTM on test implementation.The test correctly validates CLI wrapping behaviour with proper assertions for success, multi-line output, and line length constraints. Good use of
.expect()with descriptive messages.tests/wrap/mod.rs (1)
1-18: Excellent module organisation.The module documentation clearly explains the purpose and structure. The feature-based organisation into focused submodules improves maintainability and follows the coding guidelines for keeping files under 400 lines.
tests/wrap/footnotes.rs (1)
5-54: Excellent test coverage for footnote scenarios.The tests comprehensively cover footnote wrapping behaviour including multiline footnotes, inline code preservation, URL handling, and collections. Good use of helper functions and appropriate assertions.
tests/wrap/hard_line_breaks.rs (1)
5-41: Comprehensive hard line break testing.The tests thoroughly cover different hard line break scenarios including trailing spaces, backslash endings, and complex edge cases. Good use of external test data files for comprehensive validation.
tests/wrap/blockquotes.rs (7)
1-1: Excellent module documentation.The doc comment clearly explains the module's purpose and follows the required format.
5-19: Well-structured blockquote wrapping test.The test properly validates that blockquoted text wraps correctly whilst preserving the blockquote prefix structure.
21-35: Thorough nested blockquote testing.The test validates both the wrapping behaviour and content preservation for nested blockquotes using appropriate helper functions.
37-51: Good coverage of mixed indentation scenarios.The test ensures proper handling of tabs and spaces in blockquote prefixes during wrapping.
53-72: Comprehensive blank line preservation test.The test properly validates that blank lines within blockquotes are preserved during wrapping, with good use of slice indexing to test different sections.
74-88: Appropriate extra whitespace handling test.The test ensures that additional spacing in blockquote prefixes doesn't interfere with wrapping logic.
90-95: Good edge case coverage for short content.The test validates that short blockquotes that don't require wrapping remain unchanged.
tests/wrap/links.rs (7)
1-1: Clear and focused module documentation.The doc comment appropriately describes the module's specific purpose within the wrapping test suite.
5-20: Solid link preservation test.The test properly validates that Markdown links remain intact during paragraph wrapping.
22-35: Good image link coverage.The test ensures image links are preserved during wrapping, covering an important Markdown construct.
37-50: Excellent nested parentheses handling.The test validates that links containing parentheses in URLs are preserved correctly during wrapping.
62-74: Excellent use of parameterised testing.The rstest parameterisation efficiently tests multiple punctuation scenarios without code duplication. This follows best practices for comprehensive test coverage.
76-81: Good edge case coverage.The test validates links at line boundaries remain unchanged.
83-95: Comprehensive punctuation handling.Both tests validate that punctuation within link text and URLs doesn't interfere with link preservation during wrapping.
tests/wrap/lists.rs (10)
1-1: Clear module documentation.The doc comment properly explains the module's focus within the wrapping test suite.
5-12: Basic list wrapping test is sound.The test validates fundamental list item wrapping behaviour with proper prefix preservation.
14-27: Excellent parameterised testing approach.The rstest parameterisation efficiently tests multiple list prefix formats (bullet and numbered) with inline code preservation. This demonstrates good test design.
29-38: Thorough inline code span testing.The test ensures inline code spans are preserved during list item wrapping, which is critical for Markdown formatting integrity.
40-48: Good multi-backtick code coverage.The test validates that different backtick lengths for code spans are handled correctly during wrapping.
50-58: Comprehensive multiple code span testing.The test ensures multiple inline code spans within a single list item are preserved correctly.
60-76: Detailed long inline code testing.The test validates both wrapping behaviour and specific formatting preservation for complex inline code scenarios. The assertion checking the colon suffix is particularly thorough.
78-94: Precise punctuation handling test.The test uses exact string matching to validate complex punctuation scenarios within code spans, ensuring formatting integrity is maintained.
96-101: Good edge case coverage.The test validates that short list items requiring no wrapping remain unchanged.
103-149: Excellent external data file usage.The series of tests loading input and expected output from external files demonstrates good separation of test data from test logic. This approach handles complex punctuation scenarios systematically whilst keeping the test code clean.
tests/table/reflow.rs (2)
1-4: Excellent module documentation and structure.The module documentation clearly explains the purpose, and the import structure is clean. The use of
rstestfixtures promotes good test reusability.
5-32: Well-structured test coverage with good edge case handling.The tests comprehensively cover the
reflow_tablefunction behaviour including:
- Basic functionality
- Malformed input handling (returns original)
- Header preservation
- Escaped pipe character handling
- Indentation preservation
Each test is focused and uses descriptive naming. The parameterised approach with
rstestis appropriate.tests/table/regressions.rs (2)
1-4: Clear module documentation for regression testing.The module documentation accurately describes the purpose of testing with real data tables.
5-52: Confirm all regression test data files existAll referenced data files under tests/data are present and readable:
- tests/data/logical_type_input.txt
- tests/data/logical_type_expected.txt
- tests/data/option_table_input.txt
- tests/data/option_table_expected.txt
- tests/data/month_seconds_input.txt
- tests/data/month_seconds_expected.txt
- tests/data/offset_table_input.txt
- tests/data/offset_table_expected.txt
- tests/data/regression_table_input.txt
- tests/data/regression_table_expected.txt
No further action required.
tests/table/uniform.rs (1)
1-4: Clear module documentation for uniformity testing.The module documentation clearly states the purpose of testing uniform column widths.
tests/table/process_stream_tests.rs (2)
1-4: Clear module documentation for stream processing tests.The module documentation clearly explains the purpose of testing
process_streamtable behaviour.
5-87: Comprehensive test coverage with excellent edge case handling.The tests provide thorough coverage of
process_streamfunctionality:
- Parameterised testing for HTML variants using
rstest- Multiple table handling with spacing preservation
- Code fence preservation (both backtick and tilde)
- Indented code fence handling
- Mixed content processing
- Whitespace-only input normalisation
The test structure is well-organised and follows Rust testing best practices.
tests/table/convert_html.rs (2)
1-4: Clear module documentation for HTML conversion testing.The module documentation accurately describes the purpose of testing HTML table conversion to Markdown.
5-63: Excellent test organisation with comprehensive HTML table coverage.The tests provide thorough coverage of HTML table conversion:
- Parameterised testing for standard HTML variants (
basic,with_attrs,uppercase)- Separate parameterised tests for reduced output scenarios (
colspan,inconsistent)- Individual tests for specific edge cases (no header, empty row, whitespace header)
- Boundary condition testing (empty tables, unclosed tables)
- External data file integration for complex cases
The logical grouping of test cases and descriptive naming make the test suite easy to understand and maintain.
tests/table/mod.rs (3)
7-9: LGTM!The macro import and prelude usage follows Rust conventions appropriately.
17-138: LGTM on fixture content and organisation!The fixtures provide comprehensive coverage of table scenarios including edge cases like malformed tables, escaped pipes, indented tables, and various HTML table variants. The naming convention is consistent and descriptive. The use of
rstestfixtures aligns with the coding guidelines for shared setup and avoiding repetition.
140-144: LGTM on module organisation!The submodule declarations follow a logical organisation that separates concerns appropriately. This structure supports the refactoring goal of keeping files under 400 lines whilst maintaining comprehensive test coverage.
|
@sourcery-ai resolve |
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
1 participant
Summary
wrap.rstests into feature modules undertests/wrap/table.rstests into feature modules undertests/table/Testing
make fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_688b8d3bbe08832292e05015dd72ff18
Summary by Sourcery
Restructure existing wrap and table tests into organized feature-specific modules
Enhancements: