Skip to content

fix: allow jq examples in quality gate dry-runs#1979

Merged
HanShaoshuai-k merged 1 commit into
mainfrom
fix/quality-gate-jq-dry-run
Jul 21, 2026
Merged

fix: allow jq examples in quality gate dry-runs#1979
HanShaoshuai-k merged 1 commit into
mainfrom
fix/quality-gate-jq-dry-run

Conversation

@HanShaoshuai-k

@HanShaoshuai-k HanShaoshuai-k commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

The deterministic quality gate executes documented CLI examples with --dry-run to validate their request previews. Examples using -q or --jq could filter the synthetic dry-run envelope into null or fail at runtime, causing valid examples such as those in PR #1906 to be rejected.

This change validates jq-related output flags first, then removes valid jq filters only from the synthetic dry-run invocation so the underlying request preview remains available for validation.

Changes

  • Validate jq syntax and its compatibility with --format and --output before normalizing dry-run examples.
  • Remove valid -q and --jq filters from synthetic dry-run invocations while preserving the original documented command.
  • Keep malformed jq expressions, missing values, and incompatible output combinations intact so the real CLI execution still rejects them.
  • Add regression coverage for short, long, split, and inline jq forms, including malformed and conflicting cases.
  • Add an end-to-end quality-gate test based on the im +flag-list example affected in PR fix(im): warn when flag pagination is truncated #1906.

Test Plan

  • go test ./internal/qualitygate/... -count=1
  • go vet ./...
  • go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=upstream/main
  • QUALITY_GATE_CHANGED_FROM=upstream/main make quality-gate
  • go mod tidy produced no changes
  • Full local stock-example scan harvested 1,849 examples and executed 207 dry-run previews
  • Both existing im +flag-list -q ... examples produced the expected GET /open-apis/im/v1/flags request preview

The full stock-example scan also reported pre-existing unrelated reference and placeholder-validation findings outside this PR's changed files.

Related Issues

Summary by CodeRabbit

  • Bug Fixes
    • Improved dry-run request previews when commands include valid jq output filters.
    • Dry-run validation now ignores output-only filters while preserving the original command’s behavior.
    • Improved compatibility between JSON formatting and dry-run previews.
    • Invalid filters and incompatible output options continue to produce the expected CLI errors.

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jul 21, 2026
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 08e9fe8b-ab0c-405a-be66-e782cdcbe322

📥 Commits

Reviewing files that changed from the base of the PR and between ad4a6d6 and 5cdcefc.

📒 Files selected for processing (2)
  • internal/qualitygate/rules/dryrun.go
  • internal/qualitygate/rules/dryrun_test.go

📝 Walkthrough

Walkthrough

Dry-run command preparation now validates jq and output flags, removes compatible jq filters from preview invocations, conditionally normalizes pretty output to JSON, and preserves invalid combinations for deterministic CLI errors. Tests cover request previews and argument variants.

Changes

Dry-run jq handling

Layer / File(s) Summary
Validate and rewrite dry-run output arguments
internal/qualitygate/rules/dryrun.go
Dry-run argument preparation parses jq, output, and format flags, validates compatibility, strips safe jq filters, and conditionally forces JSON formatting.
Validate preview execution and argument cases
internal/qualitygate/rules/dryrun_test.go
Tests verify jq filters are omitted from executable request previews and cover split, inline, missing, invalid, and format-related argument combinations.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: domain/task

Suggested reviewers: liangshuo-1

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change to dry-run handling for jq examples.
Description check ✅ Passed The description follows the template with Summary, Changes, Test Plan, and Related Issues and is sufficiently complete.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/quality-gate-jq-dry-run

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.

@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@5cdcefcea4ef0149e91dbc8747353ed6606d2591

🧩 Skill update

npx skills add larksuite/cli#fix/quality-gate-jq-dry-run -y -g

@HanShaoshuai-k
HanShaoshuai-k merged commit 577ff03 into main Jul 21, 2026
37 of 52 checks passed
@HanShaoshuai-k
HanShaoshuai-k deleted the fix/quality-gate-jq-dry-run branch July 21, 2026 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants