Skip to content

refactor: decompose actions into testable modules#14

Merged
dortort merged 9 commits into
mainfrom
claude/refactor-actions-testing-QJgnS
Mar 28, 2026
Merged

refactor: decompose actions into testable modules#14
dortort merged 9 commits into
mainfrom
claude/refactor-actions-testing-QJgnS

Conversation

@dortort
Copy link
Copy Markdown
Owner

@dortort dortort commented Mar 28, 2026

Extract each action's monolithic index.ts into a 3-layer architecture:

  • index.ts: thin shell (reads inputs, calls run function)
  • run.ts: orchestration (accepts ActionContext + plain data, testable)
  • prompts.ts / parsers.ts / etc: pure functions

Shared infrastructure changes:

  • Add createActionContext() factory for injecting mock dependencies
  • Add @gemini-actions/shared/testing with mock factories
  • Add root vitest.config.ts and nx test target
  • Add test step to CI pipeline
  • Exclude test files from tsconfig (ncc build)

Per-action decomposition:

  • pr-review: run.ts + prompts.ts + review.ts + tests
  • test-failure-diagnosis: run.ts + parsers.ts + tests
  • pr-from-issue: run.ts + prompts.ts + tests
  • datadog-responder: run.ts + datadog.ts + prompts.ts + tests
  • repo-qa: run.ts + globs.ts + prompts.ts + tests
  • dependency-impact: run.ts + registry.ts (thin index.ts shell)

All 117 tests pass across 6 action workspaces.

https://claude.ai/code/session_01LhT9QaGdJ2J37cnBEULtL5

claude added 9 commits March 28, 2026 19:56
…t config

Add createActionContext() to build an ActionContext from pre-constructed
dependencies, enabling tests to inject mocks without touching @actions/core.

Add root vitest.config.ts and an nx test target so
`npx nx affected --target=test` works across all workspaces.

https://claude.ai/code/session_01LhT9QaGdJ2J37cnBEULtL5
Run `npx nx affected --target=test` after building. Zero-cost until
workspaces add a test script — actions without one are simply skipped.

https://claude.ai/code/session_01LhT9QaGdJ2J37cnBEULtL5
…rativeModel

Add @gemini-actions/shared/testing entry point with:
- createMockOctokit(): proxy-based stub for GitHub API calls
- createMockModel(): canned-response GenerativeModel mock
- createTestContext(): convenience wrapper combining both

Exported separately so production code never imports test helpers.

https://claude.ai/code/session_01LhT9QaGdJ2J37cnBEULtL5
Decompose monolithic index.ts into:
- run.ts: runPrReview(ctx, inputs) orchestration function
- prompts.ts: STRICTNESS_PROMPTS, buildFileSections, buildReviewPrompt
- review.ts: filterValidComments, buildSummaryBody
- index.ts: thin shell that reads inputs and delegates

Add tests for all three modules (14 tests).

https://claude.ai/code/session_01LhT9QaGdJ2J37cnBEULtL5
…tability

Decompose monolithic index.ts into:
- run.ts: runTestFailureDiagnosis(ctx, {prNumber, testOutput}) — receives
  file content as string, eliminating the fs dependency from testable code
- parsers.ts: extractTestFilePaths() pure function
- index.ts: thin shell that reads the file and delegates

Add parser tests covering Jest, pytest, Go, generic patterns (9 tests).

https://claude.ai/code/session_01LhT9QaGdJ2J37cnBEULtL5
Decompose monolithic index.ts into:
- run.ts: runPrFromIssue(ctx, {issueNumber}) orchestration
- prompts.ts: buildPlanPrompt, buildChangePrompt, buildPrBody
- index.ts: thin shell

Add prompt tests (4 tests).

https://claude.ai/code/session_01LhT9QaGdJ2J37cnBEULtL5
…testability

Decompose monolithic index.ts into:
- datadog.ts: DatadogClient interface + createDatadogClient() factory,
  making the HTTP layer injectable for tests
- run.ts: runDatadogResponder(ctx, inputs, ddClient) orchestration
- prompts.ts: buildAnalysisPrompt()
- index.ts: thin shell

Add prompt tests (5 tests).

https://claude.ai/code/session_01LhT9QaGdJ2J37cnBEULtL5
Decompose monolithic index.ts into:
- globs.ts: matchGlob, matchesAnyGlob pure functions
- prompts.ts: buildFileSelectionPrompt, buildAnswerPrompt
- run.ts: runRepoQA(ctx, inputs) orchestration
- index.ts: thin shell

Add glob matching tests (9 tests).

https://claude.ai/code/session_01LhT9QaGdJ2J37cnBEULtL5
…lity

Decompose monolithic index.ts into:
- registry.ts: RegistryResolver type + createRegistryResolver(fetchFn)
  with injectable fetch for testing
- run.ts: runDependencyImpact(ctx, inputs, resolver) orchestration
- index.ts: thin shell

Existing parsers.ts, prompts.ts, review.ts, types.ts modules and their
tests are unchanged.

https://claude.ai/code/session_01LhT9QaGdJ2J37cnBEULtL5
@dortort dortort force-pushed the claude/refactor-actions-testing-QJgnS branch from a1fd41c to 656d93f Compare March 28, 2026 19:57
@dortort dortort merged commit a7a66c8 into main Mar 28, 2026
1 check passed
@dortort dortort deleted the claude/refactor-actions-testing-QJgnS branch March 28, 2026 19:58
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1fd41c577

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +190 to +191
const parsed = parseJsonResponse<Step2Result>(step2Response);
step2Result = parsed;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Validate Step 2 JSON shape before replacing safe defaults

parseJsonResponse only guarantees syntactic JSON (see shared/src/gemini.ts), so Gemini can return objects missing impacts/unaffectedUsages without throwing. In this block, assigning step2Result = parsed immediately means malformed Step 2 output can silently replace the safe { impacts: [], unaffectedUsages: [] } fallback, and Step 3 then synthesizes risk from incomplete/incorrect structure instead of the intended empty-impact baseline. Please keep a shape check (or access required fields) before assignment so bad Step 2 payloads fall back cleanly.

Useful? React with 👍 / 👎.

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.

2 participants