Skip to content

test(doc): harden markdown_fix pipeline with invariant tests#576

Merged
fangshuyu-768 merged 1 commit intolarksuite:mainfrom
fangshuyu-768:test/markdown-fix-hardening
Apr 20, 2026
Merged

test(doc): harden markdown_fix pipeline with invariant tests#576
fangshuyu-768 merged 1 commit intolarksuite:mainfrom
fangshuyu-768:test/markdown-fix-hardening

Conversation

@fangshuyu-768
Copy link
Copy Markdown
Collaborator

@fangshuyu-768 fangshuyu-768 commented Apr 20, 2026

Summary

Follow-up to #469 (merged in 293a9f8). Adds 5 invariant-level tests that exercise the markdown-fix pipeline that #469 introduced / reworked. Pure test coverage — no production code changes.

What's added

Test Guards against
TestFixExportedMarkdownIdempotent f(f(x)) == f(x) across kitchen-sink / CJK / nested-container fixtures — the round-trip promise #469 is built on
TestFixExportedMarkdownPreservesFencedCodeByteForByte code samples silently rewritten by any transform — packs every pipeline-touching shape into a single fence and asserts byte-identical output
TestFixExportedMarkdownPreservesCRLF Windows-authored markdown being LF-normalized; also asserts transforms still fire on CRLF input
TestFixExportedMarkdownTransformInteractions composition regressions across transforms: nested-list + trailing-space bold, text→list transition, callout containing list with emphasis, heading vs paragraph bold
TestNormalizeNestedListIndentationDocumentedSkips deliberate no-op branches of normalizeNestedListIndentation (odd-space indent, blank-line loose-list sibling, 4-space indented code block, parentless 2-space indent) — freezes them as an explicit spec

Why

The existing table-driven tests in markdown_fix_test.go cover each Fix* function in isolation. These new tests target structural invariants that are easy to violate in a future refactor:

  • Idempotency — directly checks the property fix(doc): preserve round-trip formatting in fetch output #469's description calls out ("preserve round-trip formatting"). Would catch any transform that rewrites its own output across fetch → edit → update → fetch.
  • Code-fence byte-exactness — stronger than existing strings.Contains assertions: compares byte-by-byte. Pipeline's strongest invariant (user code samples must never be silently modified).
  • CRLF — first Windows-line-ending coverage in this test file; issue docs +create: Code blocks get a spurious trailing empty line 飞书文档中的代码块总是跟一个空白行 #53 was reported on Windows 11.
  • Transform interactions — individual Fix* functions are well-tested in isolation; composition was not.
  • Documented skips — locks in the current "skip" behaviour for odd-space indent etc. as an explicit spec so a future heuristic change can't silently start rewriting those shapes.

Test plan

Context

Originally opened against GaoSSR's PR branch as GaoSSR/cli#1 while #469 was still in review, so the tests could land together with the implementation. Now that #469 is merged, that PR has been closed and this is the direct follow-up against main.

Summary by CodeRabbit

  • Tests
    • Added comprehensive test coverage for markdown processing to ensure reliability and correct handling of edge cases.

Adds 5 invariant-level tests on top of larksuite#469's transforms:

- TestFixExportedMarkdownIdempotent — f(f(x)) == f(x) across rich
  fixtures (kitchen sink, CJK, nested containers). Protects the core
  round-trip promise from future transform interactions that rewrite
  their own output.
- TestFixExportedMarkdownPreservesFencedCodeByteForByte — packs every
  pipeline-touching shape into a fence and asserts byte-identical output.
  Code samples must never be silently rewritten by a formatting pass.
- TestFixExportedMarkdownPreservesCRLF — CRLF input preserves line
  endings AND still triggers transforms. Windows-authored markdown
  should not be silently LF-normalized.
- TestFixExportedMarkdownTransformInteractions — composition regressions:
  nested-list + trailing-space bold, text→list transition, callout
  containing list with emphasis, heading vs paragraph bold.
- TestNormalizeNestedListIndentationDocumentedSkips — locks in the
  deliberate no-op branches (odd-space indent, blank-line loose-list
  sibling, 4-space indented code block, parentless two-space) as an
  explicit spec so future heuristic tweaks surface in the test diff.

All transforms, fixtures, and expectations are derived from the head of
PR larksuite#469. No production code changes.
@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/S Low-risk docs, CI, test, or chore only changes labels Apr 20, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 20, 2026

📝 Walkthrough

Walkthrough

A new test file shortcuts/doc/markdown_fix_hardening_test.go was added containing a comprehensive test suite that validates the markdown fixing pipeline. Tests cover idempotency, code block content preservation, CRLF handling, composition interactions, and edge cases in normalization logic.

Changes

Cohort / File(s) Summary
Markdown Pipeline Hardening Tests
shortcuts/doc/markdown_fix_hardening_test.go
New test suite validating fixExportedMarkdown function with tests for idempotency, fenced code block content preservation, CRLF line ending handling, multiple transform interactions, and documented "do nothing" branches of normalizeNestedListIndentation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

size/M, domain/ccm

Poem

🐰 A hardening test hops into sight,
Validating markdown, making it right,
Fences preserved, indents aligned,
CRLF's kept, no glitches designed,
Idempotent paths that sparkle and gleam,
Tests lock in the truth of each codified dream!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding hardening tests for the markdown_fix pipeline. It is specific, concise, and directly reflects the primary objective of the PR.
Description check ✅ Passed The PR description follows the template structure with all required sections present: Summary, Changes (presented as a detailed table), Test Plan (with checkmarks showing verification), and Related Issues. Content is comprehensive and well-organized.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
shortcuts/doc/markdown_fix_hardening_test.go (1)

129-147: Optional: rename close local to avoid shadowing the builtin.

close is a Go built-in. Shadowing it in a helper works but is a common lint nit and reduces readability. Consider endIdx or closeIdx.

♻️ Proposed rename
-	close := strings.Index(rest, "\n"+fence)
-	if close < 0 {
+	closeIdx := strings.Index(rest, "\n"+fence)
+	if closeIdx < 0 {
 		return "", false
 	}
-	return rest[:close], true
+	return rest[:closeIdx], true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/doc/markdown_fix_hardening_test.go` around lines 129 - 147, In
extractFirstFenceContent, the local variable named close shadows the built-in
close; rename that identifier (e.g., closeIdx or endIdx) and update its single
use in the function (the search and the slice return) so the code reads closeIdx
:= strings.Index(rest, "\n"+fence) and return rest[:closeIdx], true — keep all
other logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@shortcuts/doc/markdown_fix_hardening_test.go`:
- Around line 129-147: In extractFirstFenceContent, the local variable named
close shadows the built-in close; rename that identifier (e.g., closeIdx or
endIdx) and update its single use in the function (the search and the slice
return) so the code reads closeIdx := strings.Index(rest, "\n"+fence) and return
rest[:closeIdx], true — keep all other logic unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b92bdbdd-f2fb-4e72-bf29-271dc7b9b323

📥 Commits

Reviewing files that changed from the base of the PR and between 293a9f8 and 5ecd444.

📒 Files selected for processing (1)
  • shortcuts/doc/markdown_fix_hardening_test.go

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.42%. Comparing base (293a9f8) to head (5ecd444).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #576   +/-   ##
=======================================
  Coverage   60.42%   60.42%           
=======================================
  Files         393      393           
  Lines       33657    33657           
=======================================
+ Hits        20336    20338    +2     
+ Misses      11433    11432    -1     
+ Partials     1888     1887    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Collaborator

@SunPeiYang996 SunPeiYang996 left a comment

Choose a reason for hiding this comment

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

good

@fangshuyu-768 fangshuyu-768 merged commit 9e891b7 into larksuite:main Apr 20, 2026
25 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/ccm PR touches the ccm domain size/S Low-risk docs, CI, test, or chore only changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants