Skip to content

test: fix baseline test build#152

Open
Star-sumi wants to merge 1 commit into
OlaProeis:masterfrom
Star-sumi:fix/ci-test-build
Open

test: fix baseline test build#152
Star-sumi wants to merge 1 commit into
OlaProeis:masterfrom
Star-sumi:fix/ci-test-build

Conversation

@Star-sumi

@Star-sumi Star-sumi commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • fix test-only imports needed by existing history and code execution tests
  • re-export flowchart parser helpers under #[cfg(test)] and align the back-edge test import
  • add a test-only Task::to_markdown() helper for existing productivity panel tests

Validation

  • git diff --check
  • cargo check --locked --quiet
  • cargo test --locked test_task_to_markdown -- --nocapture
  • cargo test --locked test_new_history -- --nocapture
  • cargo test --locked test_simple_undo -- --nocapture
  • cargo test --locked classify_normalizes_case -- --nocapture
  • cargo test --locked status_glyphs -- --nocapture
  • cargo test --locked test_parse_direction -- --nocapture
  • cargo test --locked fc_83a_inner_e_to_b_goes_up_first -- --nocapture

Notes

  • cargo fmt --all -- --check currently fails on pre-existing repository-wide formatting debt outside this patch, including trailing whitespace in src/editor/widget.rs.
  • This PR intentionally stays separate from PR style: refine outline tree rendering #151 because the CI failures are unrelated to the outline tree work.

Summary by CodeRabbit

  • Tests
    • Reorganized test-specific imports and utilities across multiple modules to improve test infrastructure, including conditional test-only helpers and imports that only compile during testing.

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Five small test-only additions: #[cfg(test)]-gated imports are added to history.rs, code_execution.rs, flowchart/mod.rs, and edges.rs to resolve symbols used in existing test code. A Task::to_markdown method is added to productivity_panel.rs, also #[cfg(test)], converting task state to markdown checkbox syntax.

Changes

Test compilation fixes and Task::to_markdown

Layer / File(s) Summary
cfg(test) imports and re-exports
src/editor/ferrite/history.rs, src/markdown/code_execution.rs, src/markdown/mermaid/flowchart/mod.rs, src/markdown/mermaid/flowchart/render/edges.rs
Adds #[cfg(test)]-gated imports of TextBuffer, CHECK/X icons, flowchart parser helpers (parse_direction, parse_edge_line_full, parse_node_from_text), and expand_rect so test-only code in each module compiles.
Task::to_markdown test-only method
src/ui/productivity_panel.rs
Adds Task::to_markdown(&self) -> String under #[cfg(test)] that produces - [ ]/- [x] checkbox text with ! /!! priority prefixes from the task's completed and priority fields.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 A sprinkle of #[cfg(test)] here,
A method for markdown, oh what cheer!
- [x] for done, - [ ] for not,
Priority bangs — the rabbit's got a lot!
Tests compile clean, the warren is bright ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'test: fix baseline test build' directly and specifically describes the main change: fixing test compilation issues by adding test-only imports and helper methods.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ui/productivity_panel.rs`:
- Around line 88-98: The format string in the to_markdown method has a literal
space between the checkbox and priority placeholder, which creates a
double-space when priority is empty. Fix this by modifying the format string and
priority handling so that a space only appears before the priority when the
priority string is not empty. One approach is to conditionally build the
priority string with a leading space only when priority is not an empty string,
or restructure the format arguments to eliminate the extra space.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8ddcdab4-c159-46b3-a081-f134364808a1

📥 Commits

Reviewing files that changed from the base of the PR and between 3ba085c and 6d64c0c.

📒 Files selected for processing (5)
  • src/editor/ferrite/history.rs
  • src/markdown/code_execution.rs
  • src/markdown/mermaid/flowchart/mod.rs
  • src/markdown/mermaid/flowchart/render/edges.rs
  • src/ui/productivity_panel.rs

Comment on lines +88 to +98
/// Render this task back to markdown checkbox syntax.
#[cfg(test)]
pub fn to_markdown(&self) -> String {
let checkbox = if self.completed { "[x]" } else { "[ ]" };
let priority = match self.priority {
2 => "!! ",
1 => "! ",
_ => "",
};
format!("- {} {}{}", checkbox, priority, self.text)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix the format string to avoid double-space when priority is zero.

The current format string format!("- {} {}{}", checkbox, priority, self.text) inserts a literal space between the checkbox and priority. When priority is empty (""), this creates a double-space before the text:

  • Produces: "- [ ] Test" (two spaces before "Test")
  • Test expects (Line 1500): "- [ ] Test" (one space)
🐛 Proposed fix
     #[cfg(test)]
     pub fn to_markdown(&self) -> String {
         let checkbox = if self.completed { "[x]" } else { "[ ]" };
         let priority = match self.priority {
-            2 => "!! ",
-            1 => "! ",
-            _ => "",
+            2 => " !! ",
+            1 => " ! ",
+            _ => " ",
         };
-        format!("- {} {}{}", checkbox, priority, self.text)
+        format!("- {}{}{}", checkbox, priority, self.text)
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Render this task back to markdown checkbox syntax.
#[cfg(test)]
pub fn to_markdown(&self) -> String {
let checkbox = if self.completed { "[x]" } else { "[ ]" };
let priority = match self.priority {
2 => "!! ",
1 => "! ",
_ => "",
};
format!("- {} {}{}", checkbox, priority, self.text)
}
/// Render this task back to markdown checkbox syntax.
#[cfg(test)]
pub fn to_markdown(&self) -> String {
let checkbox = if self.completed { "[x]" } else { "[ ]" };
let priority = match self.priority {
2 => " !! ",
1 => " ! ",
_ => " ",
};
format!("- {}{}{}", checkbox, priority, self.text)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ui/productivity_panel.rs` around lines 88 - 98, The format string in the
to_markdown method has a literal space between the checkbox and priority
placeholder, which creates a double-space when priority is empty. Fix this by
modifying the format string and priority handling so that a space only appears
before the priority when the priority string is not empty. One approach is to
conditionally build the priority string with a leading space only when priority
is not an empty string, or restructure the format arguments to eliminate the
extra space.

@Star-sumi

Copy link
Copy Markdown
Contributor Author

Fresh CI follow-up from local triage:

  • Both CI / Test jobs are red after this PR fixes the earlier test-compilation blockers.
  • The current failing tests are outside this PR's touched files.
  • Linux failures:
    • markdown::mermaid::tests::test_subgraph_title_width_expansion
    • markdown::video_embed::tests::document_parses_bare_youtube_url_paragraph
    • state::tests::test_recovery_identity_original_bleeding_repro_rejected
    • state::tests::test_recovery_identity_path_mismatch_rejected
  • Windows reports the same four plus:
    • vcs::git::tests::test_git_service_untracked_file

Local follow-up on this branch still shows the touched-file validation passing:

  • cargo check --locked --quiet
  • cargo test --locked test_task_to_markdown -- --nocapture
  • cargo test --locked test_new_history -- --nocapture
  • cargo test --locked test_simple_undo -- --nocapture
  • cargo test --locked classify_normalizes_case -- --nocapture
  • cargo test --locked status_glyphs -- --nocapture
  • cargo test --locked test_parse_direction -- --nocapture
  • cargo test --locked fc_83a_inner_e_to_b_goes_up_first -- --nocapture

I am keeping #152 narrow rather than mixing these unrelated runtime baseline failures into it. If full green CI is required for merge, the next step should be one or more follow-up baseline PRs for the runtime failures above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant