Add regression test for trailing period in list items#131
Conversation
Reviewer's GuideEnhance the line-wrapping logic to preserve trailing punctuation after inline code spans and add a regression test with fixtures to cover list items ending in a period. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbitBug Fixes
Tests
WalkthroughIntroduce a conditional in the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Runner
participant Wrap as wrap_preserving_code
participant Files as Input/Expected Files
Test->>Files: Read bullet_full_stop_input.txt
Test->>Wrap: Call process_stream(input)
Wrap->>Wrap: Iterate tokens
alt Punctuation after inline code
Wrap->>Wrap: Append punctuation to previous line
else Normal token
Wrap->>Wrap: Standard wrapping
end
Wrap-->>Test: Return wrapped output
Test->>Files: Read bullet_full_stop_expected.txt
Test->>Test: Assert output == expected
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 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 (
|
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.rs:483` </location>
<code_context>
+/// The period following the inline code span should remain on the same line
+/// as the code block rather than being wrapped onto a new indented line.
+#[test]
+fn test_wrap_list_item_period_after_code() {
+ let input: Vec<String> = include_lines!("data/bullet_full_stop_input.txt");
+ let expected: Vec<String> = include_lines!("data/bullet_full_stop_expected.txt");
+ let output = process_stream(&input);
+ assert_eq!(output, expected);
+}
</code_context>
<issue_to_address>
Consider adding additional edge cases for other punctuation marks after code spans.
Please add regression tests for other punctuation marks handled by the implementation, such as '?', '!', ',', ':', and ';', to ensure full coverage.
Suggested implementation:
```rust
#[test]
fn test_wrap_list_item_period_after_code() {
let input: Vec<String> = include_lines!("data/bullet_full_stop_input.txt");
let expected: Vec<String> = include_lines!("data/bullet_full_stop_expected.txt");
let output = process_stream(&input);
assert_eq!(output, expected);
}
/// Regression test for wrapping list items that end with a question mark.
///
/// The question mark following the inline code span should remain on the same line
/// as the code block rather than being wrapped onto a new indented line.
#[test]
fn test_wrap_list_item_question_mark_after_code() {
let input: Vec<String> = include_lines!("data/bullet_question_mark_input.txt");
let expected: Vec<String> = include_lines!("data/bullet_question_mark_expected.txt");
let output = process_stream(&input);
assert_eq!(output, expected);
}
/// Regression test for wrapping list items that end with an exclamation mark.
///
/// The exclamation mark following the inline code span should remain on the same line
/// as the code block rather than being wrapped onto a new indented line.
#[test]
fn test_wrap_list_item_exclamation_mark_after_code() {
let input: Vec<String> = include_lines!("data/bullet_exclamation_mark_input.txt");
let expected: Vec<String> = include_lines!("data/bullet_exclamation_mark_expected.txt");
let output = process_stream(&input);
assert_eq!(output, expected);
}
/// Regression test for wrapping list items that end with a comma.
///
/// The comma following the inline code span should remain on the same line
/// as the code block rather than being wrapped onto a new indented line.
#[test]
fn test_wrap_list_item_comma_after_code() {
let input: Vec<String> = include_lines!("data/bullet_comma_input.txt");
let expected: Vec<String> = include_lines!("data/bullet_comma_expected.txt");
let output = process_stream(&input);
assert_eq!(output, expected);
}
/// Regression test for wrapping list items that end with a colon.
///
/// The colon following the inline code span should remain on the same line
/// as the code block rather than being wrapped onto a new indented line.
#[test]
fn test_wrap_list_item_colon_after_code() {
let input: Vec<String> = include_lines!("data/bullet_colon_input.txt");
let expected: Vec<String> = include_lines!("data/bullet_colon_expected.txt");
let output = process_stream(&input);
assert_eq!(output, expected);
}
/// Regression test for wrapping list items that end with a semicolon.
///
/// The semicolon following the inline code span should remain on the same line
/// as the code block rather than being wrapped onto a new indented line.
#[test]
fn test_wrap_list_item_semicolon_after_code() {
let input: Vec<String> = include_lines!("data/bullet_semicolon_input.txt");
let expected: Vec<String> = include_lines!("data/bullet_semicolon_expected.txt");
let output = process_stream(&input);
assert_eq!(output, expected);
}
```
You will need to add the corresponding test data files:
- `data/bullet_question_mark_input.txt` and `data/bullet_question_mark_expected.txt`
- `data/bullet_exclamation_mark_input.txt` and `data/bullet_exclamation_mark_expected.txt`
- `data/bullet_comma_input.txt` and `data/bullet_comma_expected.txt`
- `data/bullet_colon_input.txt` and `data/bullet_colon_expected.txt`
- `data/bullet_semicolon_input.txt` and `data/bullet_semicolon_expected.txt`
Each input file should contain a list item with an inline code span followed by the respective punctuation mark, and each expected file should contain the correctly wrapped output.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (9)
tests/data/bullet_question_mark_input.txt (1)
2-4: Same wording issue as earlier fileApply the wording tweak suggested for
bullet_colon_expected.txtto maintain consistency across all test-data snippets.tests/data/bullet_comma_input.txt (1)
2-4: Same wording issue as earlier fileApply the wording tweak suggested for
bullet_colon_expected.txtto maintain consistency across all test-data snippets.tests/data/bullet_comma_expected.txt (1)
2-4: Same wording issue as earlier fileApply the wording tweak suggested for
bullet_colon_expected.txtto maintain consistency across all test-data snippets.tests/data/bullet_semicolon_input.txt (1)
2-4: Same wording issue as earlier fileApply the wording tweak suggested for
bullet_colon_expected.txtto maintain consistency across all test-data snippets.tests/data/bullet_question_mark_expected.txt (1)
3-3: Same stray “10” artefact as noted earlier
Issue and fix identical to the previous file.tests/data/bullet_colon_input.txt (1)
3-3: Same stray “10” artefact as noted earlier
Issue and fix identical to the previous file.tests/data/bullet_exclamation_mark_expected.txt (1)
3-3: Same stray “10” artefact as noted earlier
Issue and fix identical to the previous file.tests/data/bullet_semicolon_expected.txt (1)
3-3: Same stray “10” artefact as noted earlier
Issue and fix identical to the previous file.tests/wrap.rs (1)
478-548: Consolidate duplicated tests using parameterised testing.These regression tests correctly implement the previous feedback and provide comprehensive coverage for all punctuation marks. However, the identical structure creates significant code duplication that violates DRY principles.
Replace the six individual tests with a single parameterised test:
-/// Regression test for wrapping list items that end with a full stop. -/// -/// The period following the inline code span should remain on the same line -/// as the code block rather than being wrapped onto a new indented line. -#[test] -fn test_wrap_list_item_period_after_code() { - let input: Vec<String> = include_lines!("data/bullet_full_stop_input.txt"); - let expected: Vec<String> = include_lines!("data/bullet_full_stop_expected.txt"); - let output = process_stream(&input); - assert_eq!(output, expected); -} - -/// Regression test for wrapping list items that end with a question mark. -/// -/// The question mark following the inline code span should remain on the same line -/// as the code block rather than being wrapped onto a new indented line. -#[test] -fn test_wrap_list_item_question_mark_after_code() { - let input: Vec<String> = include_lines!("data/bullet_question_mark_input.txt"); - let expected: Vec<String> = include_lines!("data/bullet_question_mark_expected.txt"); - let output = process_stream(&input); - assert_eq!(output, expected); -} - -/// Regression test for wrapping list items that end with an exclamation mark. -/// -/// The exclamation mark following the inline code span should remain on the same line -/// as the code block rather than being wrapped onto a new indented line. -#[test] -fn test_wrap_list_item_exclamation_mark_after_code() { - let input: Vec<String> = include_lines!("data/bullet_exclamation_mark_input.txt"); - let expected: Vec<String> = include_lines!("data/bullet_exclamation_mark_expected.txt"); - let output = process_stream(&input); - assert_eq!(output, expected); -} - -/// Regression test for wrapping list items that end with a comma. -/// -/// The comma following the inline code span should remain on the same line -/// as the code block rather than being wrapped onto a new indented line. -#[test] -fn test_wrap_list_item_comma_after_code() { - let input: Vec<String> = include_lines!("data/bullet_comma_input.txt"); - let expected: Vec<String> = include_lines!("data/bullet_comma_expected.txt"); - let output = process_stream(&input); - assert_eq!(output, expected); -} - -/// Regression test for wrapping list items that end with a colon. -/// -/// The colon following the inline code span should remain on the same line -/// as the code block rather than being wrapped onto a new indented line. -#[test] -fn test_wrap_list_item_colon_after_code() { - let input: Vec<String> = include_lines!("data/bullet_colon_input.txt"); - let expected: Vec<String> = include_lines!("data/bullet_colon_expected.txt"); - let output = process_stream(&input); - assert_eq!(output, expected); -} - -/// Regression test for wrapping list items that end with a semicolon. -/// -/// The semicolon following the inline code span should remain on the same line -/// as the code block rather than being wrapped onto a new indented line. -#[test] -fn test_wrap_list_item_semicolon_after_code() { - let input: Vec<String> = include_lines!("data/bullet_semicolon_input.txt"); - let expected: Vec<String> = include_lines!("data/bullet_semicolon_expected.txt"); - let output = process_stream(&input); - assert_eq!(output, expected); -} +/// Regression test for wrapping list items that end with punctuation after code. +/// +/// Punctuation following inline code spans should remain on the same line +/// as the code block rather than being wrapped onto a new indented line. +#[rstest] +#[case("full_stop", "period")] +#[case("question_mark", "question mark")] +#[case("exclamation_mark", "exclamation mark")] +#[case("comma", "comma")] +#[case("colon", "colon")] +#[case("semicolon", "semicolon")] +fn test_wrap_list_item_punctuation_after_code( + #[case] file_suffix: &str, + #[case] punctuation_name: &str, +) { + let input: Vec<String> = include_lines!(&format!("data/bullet_{file_suffix}_input.txt")); + let expected: Vec<String> = include_lines!(&format!("data/bullet_{file_suffix}_expected.txt")); + let output = process_stream(&input); + assert_eq!(output, expected, "Failed to wrap list item with {punctuation_name} after code"); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
tests/data/bullet_colon_expected.txt(1 hunks)tests/data/bullet_colon_input.txt(1 hunks)tests/data/bullet_comma_expected.txt(1 hunks)tests/data/bullet_comma_input.txt(1 hunks)tests/data/bullet_exclamation_mark_expected.txt(1 hunks)tests/data/bullet_exclamation_mark_input.txt(1 hunks)tests/data/bullet_question_mark_expected.txt(1 hunks)tests/data/bullet_question_mark_input.txt(1 hunks)tests/data/bullet_semicolon_expected.txt(1 hunks)tests/data/bullet_semicolon_input.txt(1 hunks)tests/wrap.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Comments must use en-GB-oxendict spelling and grammar.
Function documentation must include clear examples.
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.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Clippy warnings MUST be disallowed.
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.
Use predicate functions for conditional criteria with more than two branches.
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.
Comment why, not what. Explain assumptions, edge cases, trade-offs, or complexity. Don't echo the obvious.
Use functions and composition. Avoid repetition by extracting reusable logic. Prefer generators or comprehensions, and declarative code to imperative repetition when readable.
Small, meaningful functions. Functions must be small, clear in purpose, single responsibility, and obey command/query segregation.
Name things precisely. Use clear, descriptive variable and function names. For booleans, prefer names withis,has, orshould.
Structure log...
Files:
tests/wrap.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.rs
🧠 Learnings (10)
tests/data/bullet_colon_expected.txt (1)
Learnt from: CR
PR: leynos/lille#0
File: docs/bevy-headless-testing.md:0-0
Timestamp: 2025-07-20T12:03:04.107Z
Learning: Applies to docs/**/*.{rs} : Use StateScoped(MyState::Variant) components to ensure entities are automatically despawned when exiting a state, improving test cleanup
tests/data/bullet_colon_input.txt (1)
Learnt from: CR
PR: leynos/lille#0
File: docs/bevy-headless-testing.md:0-0
Timestamp: 2025-07-20T12:03:04.107Z
Learning: Applies to docs/**/*.{rs} : Use StateScoped(MyState::Variant) components to ensure entities are automatically despawned when exiting a state, improving test cleanup
tests/data/bullet_exclamation_mark_input.txt (1)
Learnt from: CR
PR: leynos/lille#0
File: docs/bevy-headless-testing.md:0-0
Timestamp: 2025-07-20T12:03:04.107Z
Learning: Applies to docs/**/*.{rs} : Use StateScoped(MyState::Variant) components to ensure entities are automatically despawned when exiting a state, improving test cleanup
tests/data/bullet_semicolon_expected.txt (1)
Learnt from: CR
PR: leynos/lille#0
File: docs/bevy-headless-testing.md:0-0
Timestamp: 2025-07-20T12:03:04.107Z
Learning: Applies to docs/**/*.{rs} : Use StateScoped(MyState::Variant) components to ensure entities are automatically despawned when exiting a state, improving test cleanup
tests/data/bullet_semicolon_input.txt (1)
Learnt from: CR
PR: leynos/lille#0
File: docs/bevy-headless-testing.md:0-0
Timestamp: 2025-07-20T12:03:04.107Z
Learning: Applies to docs/**/*.{rs} : Use StateScoped(MyState::Variant) components to ensure entities are automatically despawned when exiting a state, improving test cleanup
tests/data/bullet_question_mark_expected.txt (1)
Learnt from: CR
PR: leynos/lille#0
File: docs/bevy-headless-testing.md:0-0
Timestamp: 2025-07-20T12:03:04.107Z
Learning: Applies to docs/**/*.{rs} : Use StateScoped(MyState::Variant) components to ensure entities are automatically despawned when exiting a state, improving test cleanup
tests/data/bullet_comma_expected.txt (1)
Learnt from: CR
PR: leynos/lille#0
File: docs/bevy-headless-testing.md:0-0
Timestamp: 2025-07-20T12:03:04.107Z
Learning: Applies to docs/**/*.{rs} : Use StateScoped(MyState::Variant) components to ensure entities are automatically despawned when exiting a state, improving test cleanup
tests/data/bullet_exclamation_mark_expected.txt (1)
Learnt from: CR
PR: leynos/lille#0
File: docs/bevy-headless-testing.md:0-0
Timestamp: 2025-07-20T12:03:04.107Z
Learning: Applies to docs/**/*.{rs} : Use StateScoped(MyState::Variant) components to ensure entities are automatically despawned when exiting a state, improving test cleanup
tests/data/bullet_question_mark_input.txt (1)
Learnt from: CR
PR: leynos/lille#0
File: docs/bevy-headless-testing.md:0-0
Timestamp: 2025-07-20T12:03:04.107Z
Learning: Applies to docs/**/*.{rs} : Use StateScoped(MyState::Variant) components to ensure entities are automatically despawned when exiting a state, improving test cleanup
tests/data/bullet_comma_input.txt (1)
Learnt from: CR
PR: leynos/lille#0
File: docs/bevy-headless-testing.md:0-0
Timestamp: 2025-07-20T12:03:04.107Z
Learning: Applies to docs/**/*.{rs} : Use StateScoped(MyState::Variant) components to ensure entities are automatically despawned when exiting a state, improving test cleanup
🧬 Code Graph Analysis (1)
tests/wrap.rs (1)
src/process.rs (1)
process_stream(206-214)
🪛 LanguageTool
tests/data/bullet_colon_expected.txt
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: - StateScoped Entities: A best practi...
(DASH_RULE)
[style] ~1-~1: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...practice for managing entity lifecycles in conjunction with states is the use of `StateScoped(MySta...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
tests/data/bullet_colon_input.txt
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: - StateScoped Entities: A best practi...
(DASH_RULE)
[style] ~1-~1: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...practice for managing entity lifecycles in conjunction with states is the use of `StateScoped(MySta...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
tests/data/bullet_exclamation_mark_input.txt
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: - StateScoped Entities: A best practi...
(DASH_RULE)
[style] ~1-~1: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...practice for managing entity lifecycles in conjunction with states is the use of `StateScoped(MySta...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
tests/data/bullet_semicolon_expected.txt
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: - StateScoped Entities: A best practi...
(DASH_RULE)
[style] ~1-~1: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...practice for managing entity lifecycles in conjunction with states is the use of `StateScoped(MySta...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
tests/data/bullet_semicolon_input.txt
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: - StateScoped Entities: A best practi...
(DASH_RULE)
[style] ~1-~1: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...practice for managing entity lifecycles in conjunction with states is the use of `StateScoped(MySta...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
tests/data/bullet_question_mark_expected.txt
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: - StateScoped Entities: A best practi...
(DASH_RULE)
[style] ~1-~1: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...practice for managing entity lifecycles in conjunction with states is the use of `StateScoped(MySta...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
tests/data/bullet_comma_expected.txt
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: - StateScoped Entities: A best practi...
(DASH_RULE)
[style] ~1-~1: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...practice for managing entity lifecycles in conjunction with states is the use of `StateScoped(MySta...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
tests/data/bullet_exclamation_mark_expected.txt
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: - StateScoped Entities: A best practi...
(DASH_RULE)
[style] ~1-~1: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...practice for managing entity lifecycles in conjunction with states is the use of `StateScoped(MySta...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
tests/data/bullet_question_mark_input.txt
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: - StateScoped Entities: A best practi...
(DASH_RULE)
[style] ~1-~1: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...practice for managing entity lifecycles in conjunction with states is the use of `StateScoped(MySta...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
tests/data/bullet_comma_input.txt
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: - StateScoped Entities: A best practi...
(DASH_RULE)
[style] ~1-~1: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...practice for managing entity lifecycles in conjunction with states is the use of `StateScoped(MySta...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
⏰ 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
Summary
Testing
make lintmake testhttps://chatgpt.com/codex/tasks/task_e_68889fa6ea8c832282c274fcbddc08f0
Summary by Sourcery
Prevent line wrapping from splitting trailing punctuation after inline code spans and add a regression test for list items ending with a trailing period
Bug Fixes:
Tests: