Skip to content

feat(test): optimize cli-e2e-testcase-writer skill#447

Merged
yaozhen00 merged 3 commits intomainfrom
feat/e2e_test_coverage
Apr 13, 2026
Merged

feat(test): optimize cli-e2e-testcase-writer skill#447
yaozhen00 merged 3 commits intomainfrom
feat/e2e_test_coverage

Conversation

@yaozhen00
Copy link
Copy Markdown
Collaborator

@yaozhen00 yaozhen00 commented Apr 13, 2026

Summary

optimize cli-e2e-testcase-writer skill, Generate a coverage report while writing the test case
add dorny/test-reporter for CLI E2E reports

Changes

  • optimize cli-e2e-testcase-writer skill add coverage.md
  • add dorny/test-reporter for CLI E2E reports

Test Plan

  • Unit tests pass
  • coverage report review

Related Issues

  • None

Summary by CodeRabbit

  • Chores

    • Improved CI: expanded workflow permissions, more robust E2E retries, standardized test invocation, and replaced custom report parsing with a test-report publishing step.
    • Ignored local app log file to reduce noise.
  • Documentation

    • Added comprehensive CLI E2E testing guidelines and stricter workflow/coverage rules, plus demo and task coverage templates to guide test authors.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1b06899d-4e18-4d8f-8e3b-f72c90e3dd97

📥 Commits

Reviewing files that changed from the base of the PR and between 8ca29da and 1c3535f.

📒 Files selected for processing (1)
  • .github/workflows/cli-e2e.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/cli-e2e.yml

📝 Walkthrough

Walkthrough

Updated CI workflow permissions and test reporting, adjusted gotestsum retries and package argument handling; added app.log to .gitignore; and reworked CLI E2E guidance with new per-domain coverage.md templates and stricter testcase rules.

Changes

Cohort / File(s) Summary
CI/CD Workflow
.github/workflows/cli-e2e.yml
Expanded top-level permissions (actions: read, checks: write), changed gotestsum invocation to compute packages_arg and add --rerun-fails=2 --rerun-fails-max-failures=20, and replaced a custom XML summary step with dorny/test-reporter publishing cli-e2e-report.xml as java-junit.
Repository Ignore
.gitignore
Added app.log to ignored files.
CLI E2E Documentation & Coverage
tests/cli_e2e/cli-e2e-testcase-writer/SKILL.md, tests/cli_e2e/demo/coverage.md, tests/cli_e2e/task/coverage.md
Rewrote SKILL guidance to require one domain per run and two artifacts (workflow tests + coverage.md); defined leaf-command counting and assertion rules; added demo and task coverage.md templates with command tables and coverage notes.

Sequence Diagram(s)

sequenceDiagram
    participant Actions as "GitHub Actions"
    participant Runner as "Workflow Runner"
    participant GoTest as "gotestsum"
    participant Reporter as "dorny/test-reporter"

    Actions->>Runner: trigger cli-e2e job
    Runner->>GoTest: compute packages_arg, run gotestsum (--format testname, --junitfile cli-e2e-report.xml, --rerun-fails=2, --rerun-fails-max-failures=20, --packages="$packages_arg", -count=1 -v -- ...)
    GoTest-->>Runner: emit cli-e2e-report.xml
    Runner->>Reporter: publish cli-e2e-report.xml (format: java-junit, use-actions-summary: true, list-suites/tests: all)
    Reporter-->>Actions: produce summary & annotations
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • liangshuo-1
  • kongenpei

Poem

🐰 I hopped through workflows, xml in paw,
I nudged gotestsum to try once more,
I left a coverage sheet for each domain,
Ignored the noisy app.log rain,
Tests report home — I thump and adore! 🥕✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(test): optimize cli-e2e-testcase-writer skill' clearly summarizes the main change—optimizing the CLI E2E testcase writer skill as documented in the SKILL.md updates and related test coverage additions.
Description check ✅ Passed The description includes all required template sections (Summary, Changes, Test Plan, Related Issues) with substantive content for each, though the Changes section is somewhat brief.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/e2e_test_coverage

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

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Apr 13, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 13, 2026

Greptile Summary

This PR improves the CLI E2E CI pipeline by replacing the hand-rolled Python XML summarizer with dorny/test-reporter and adding --rerun-fails=2 retry logic via gotestsum. It also refines the cli-e2e-testcase-writer skill and introduces per-domain coverage.md tracking files (a demo template and the real task domain file).

Confidence Score: 5/5

Safe to merge; only a minor cleanup suggestion remains.

All findings are P2. The unused setup-python step is a minor inefficiency but does not affect correctness or security. The dorny/test-reporter action is properly pinned to a commit SHA, permissions are appropriately scoped, and the retry logic is a sound improvement.

.github/workflows/cli-e2e.yml — the setup-python step can be removed since the Python summarizer was deleted.

Important Files Changed

Filename Overview
.github/workflows/cli-e2e.yml Replaces custom Python XML summarizer with dorny/test-reporter; adds --rerun-fails retry logic; leaves a now-unused setup-python step.
.gitignore Adds app.log to gitignore; straightforward, no issues.
tests/cli_e2e/cli-e2e-testcase-writer/SKILL.md Streamlines skill description to focus on one-domain-per-run workflow and introduces the coverage.md maintenance requirement; no issues.
tests/cli_e2e/demo/coverage.md New template file illustrating the expected coverage.md format for real domains; clearly marked as demo/reference only.
tests/cli_e2e/task/coverage.md New coverage tracking file for the task domain; denominator (29), covered count (10), and 34.5% rate are internally consistent with the command table.

Sequence Diagram

sequenceDiagram
    participant GH as GitHub Actions
    participant GT as gotestsum
    participant TR as dorny/test-reporter

    GH->>GT: go run gotestsum --rerun-fails=2 --packages=... --junitfile cli-e2e-report.xml
    GT-->>GT: Run tests (up to 3 attempts for failing tests)
    GT->>GH: cli-e2e-report.xml (JUnit XML)
    GH->>TR: uses: dorny/test-reporter (if: !cancelled())
    TR->>GH: Publish check run + Actions summary
Loading

Reviews (3): Last reviewed commit: "feat(test): test report show" | Re-trigger Greptile

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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/cli_e2e/cli-e2e-testcase-writer/SKILL.md`:
- Line 107: Update the README text that reads "Write testcase entries in `go
test -run` friendly form." to use a hyphenated compound modifier for clarity by
changing it to "Write testcase entries in `go test -run`-friendly form."; locate
the exact string in SKILL.md and replace it accordingly.
🪄 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: 179b9c5c-006f-4ca5-a9d3-201a8ea3d864

📥 Commits

Reviewing files that changed from the base of the PR and between bb79572 and 5878a90.

📒 Files selected for processing (5)
  • .github/workflows/cli-e2e.yml
  • .gitignore
  • tests/cli_e2e/cli-e2e-testcase-writer/SKILL.md
  • tests/cli_e2e/demo/coverage.md
  • tests/cli_e2e/task/coverage.md

Comment thread tests/cli_e2e/cli-e2e-testcase-writer/SKILL.md
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 13, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@1c3535f78321c161ec2ae28e0da5515db772d77c

🧩 Skill update

npx skills add larksuite/cli#feat/e2e_test_coverage -y -g

@yaozhen00 yaozhen00 force-pushed the feat/e2e_test_coverage branch from 5878a90 to 8ca29da Compare April 13, 2026 11:01
@yaozhen00 yaozhen00 force-pushed the feat/e2e_test_coverage branch from 8ca29da to 1c3535f Compare April 13, 2026 11:16
@yaozhen00 yaozhen00 merged commit c2b1329 into main Apr 13, 2026
12 checks passed
@yaozhen00 yaozhen00 deleted the feat/e2e_test_coverage branch April 13, 2026 13:10
yxzhaao pushed a commit to yxzhaao/cli that referenced this pull request Apr 14, 2026
* feat(test): optimize cli-e2e-testcase-writer skill add coverage.md

* feat(test): test report show
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