Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

test(doc): derive fetch test flag defaults from v2FetchFlags#1428

Open
vegerot wants to merge 1 commit into
larksuite:main from
vegerot:pr1428
Open

test(doc): derive fetch test flag defaults from v2FetchFlags #1428
vegerot wants to merge 1 commit into
larksuite:main from
vegerot:pr1428

Conversation

@vegerot

@vegerot vegerot commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Replace hardcoded flag defaults in the fetch test helpers with
fetchDefault() / fetchDefaultInt() helpers that read the declared
defaults from v2FetchFlags(). This prevents future drift between
production flag defaults and test setup, and panics loudly if a
flag name is misspelled rather than silently returning "".

The tests now correctly avoid hardcoding doc-format, but other
flag defaults (detail, revision-id, scope, etc.) were still
duplicated here. Deriving all defaults from v2FetchFlags() keeps
the whole test command definition aligned with production.

Co-authored-by: TraeCli (Doubao-Seed-Dogfooding) trae@bytedance.com

Stack created with Sapling. Best reviewed with ReviewStack.

 

Replace hardcoded flag defaults in the fetch test helpers with
fetchDefault() / fetchDefaultInt() helpers that read the declared
defaults from v2FetchFlags(). This prevents future drift between
production flag defaults and test setup, and panics loudly if a
flag name is misspelled rather than silently returning "".
The tests now correctly avoid hardcoding doc-format, but other
flag defaults (detail, revision-id, scope, etc.) were still
duplicated here. Deriving all defaults from v2FetchFlags() keeps
the whole test command definition aligned with production.
Co-authored-by: TraeCli (Doubao-Seed-Dogfooding) <trae@bytedance.com>
Copilot AI review requested due to automatic review settings June 12, 2026 04:10

coderabbitai Bot commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

i️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 48086d5f-ad78-42ec-b6ba-e3ce445553a1

📥 Commits

Reviewing files that changed from the base of the PR and between 76ba6fa and 826a3e8.

📒 Files selected for processing (1)
  • shortcuts/doc/docs_fetch_v2_test.go
📝 Walkthrough

Walkthrough

This PR refactors test infrastructure in a single file to reduce duplication of flag defaults. It introduces helper functions that extract default values dynamically from v2 flag definitions, then updates two test runtime builders to use those helpers instead of hardcoded literal values.

Changes

Test Fixture Default Extraction

Layer / File(s) Summary
Default extraction helpers and test runtime updates
shortcuts/doc/docs_fetch_v2_test.go		 Adds fmt import and introduces fetchDefault and fetchDefaultInt helper functions that look up flag defaults from v2FetchFlags. Updates newFetchBodyTestRuntime to construct the +fetch command using extracted defaults, and refactors newFetchShortcutTestRuntime to replace hardcoded defaults for doc-format, detail, revision-id, scope, context-before, context-after, and max-depth flags with calls to the new helpers.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A rabbit hops through test defaults with glee,
No more hardcoded numbers to see,
Helpers extract what the flags truly know,
One source of truth for the test runtime show! 🐰✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description covers the motivation and main changes, but lacks a dedicated Test Plan section and Related Issues section as specified in the template. Add a Test Plan section describing how the changes were verified and a Related Issues section (even if just 'None'). Ensure the description follows the template structure more closely.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: deriving fetch test flag defaults from v2FetchFlags instead of hardcoding them.
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.

@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 Jun 12, 2026

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates v2 docs fetch tests to derive flag defaults from the real v2FetchFlags() so tests don’t hardcode (and drift from) production defaults.

Changes:

  • Replace hardcoded flag default values in test runtime setup with values pulled from v2FetchFlags().
  • Add fetchDefault / fetchDefaultInt helpers to look up and parse defaults.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread shortcuts/doc/docs_fetch_v2_test.go
Comment thread shortcuts/doc/docs_fetch_v2_test.go
Comment on lines +158 to +165
func fetchDefault(name string) string {
for _, fl := range v2FetchFlags() {
if fl.Name == name {
return fl.Default
}
}
panic(fmt.Sprintf("fetchDefault: flag %q not found in v2FetchFlags", name))
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /