Skip to content

refactor(project): clean up project handler tests to use common utils - #2256

Open
Hweinstock wants to merge 4 commits into
aws:refactorfrom
Hweinstock:test/dedupe-project-test-helpers
Open

refactor(project): clean up project handler tests to use common utils#2256
Hweinstock wants to merge 4 commits into
aws:refactorfrom
Hweinstock:test/dedupe-project-test-helpers

Conversation

@Hweinstock

Copy link
Copy Markdown
Contributor

Problem

Each project test redefined their own version of the same utilities.

Examples:

Solution

  • standardize the temp directory helper, and project initiation in src/testing/projects.ts.

Future Work

  • Hoping to also migrate to a more consistent model where handler tests live next to the handler rather than project.test.ts

@github-actions github-actions Bot added the size/xl PR size: XL label Sep 9, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added agentcore-harness-reviewing AgentCore Harness review in progress claude-security-reviewing Claude Code /security-review in progress labels Sep 9, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed claude-security-reviewing Claude Code /security-review in progress agentcore-harness-reviewing AgentCore Harness review in progress labels Sep 9, 2026

@agentcore-devx-automation agentcore-devx-automation Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AgentCore Harness Review

Verdict: Looks good

Nice consolidation. Extracting inTempDirectory and initProject into src/testing/ and replacing the per-file originalCwd/tempDirectories boilerplate with a cleanups list makes the tests substantially shorter and more consistent. The refactor is test-only, no production code changes, and it stays on the "real temp directory over fs mock" side of the mocking guidance.

A few minor observations — none blocking:

  • src/testing/projects.ts: if root.route([... "create" ...]) throws inside initProject, the temp directory created by inTempDirectory is leaked and cwd isn't restored, since the returned cleanup hasn't reached the caller yet. The previous pattern pushed the temp dir onto a shared array before run, so afterEach still cleaned it. Only matters for failure paths of project create itself, but a try { … } catch { await cleanup(); throw } inside initProject would preserve the old behavior.
  • src/handlers/project/project.test.ts: the const cleanups = [] declaration now sits below two tests that reference it (project status requires an AgentCore project, project dev requires an AgentCore project). Works because the test bodies run after module init, but it reads a little surprising — worth hoisting the declaration to the top of the file next to the other module-level state.
  • Files with both afterEach(cleanupScreens) and afterEach(() => Promise.all(cleanups…)) rely on registration-order execution. That's what Bun does today, and it matches the prior behavior, so fine as-is — just something to keep in mind if we ever move to another runner.

Happy to see this land.

@codecov-commenter

codecov-commenter commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.04%. Comparing base (4fe2c10) to head (2e2c8d4).

Additional details and impacted files
@@             Coverage Diff              @@
##           refactor    #2256      +/-   ##
============================================
- Coverage     97.04%   97.04%   -0.01%     
============================================
  Files           566      566              
  Lines         39353    39334      -19     
============================================
- Hits          38190    38171      -19     
  Misses         1163     1163              

☔ View full report in Codecov by Harness.
📢 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.

@github-actions github-actions Bot added size/xl PR size: XL and removed size/xl PR size: XL labels Sep 9, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Sep 9, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Sep 9, 2026
@github-actions github-actions Bot added size/xl PR size: XL and removed size/xl PR size: XL labels Sep 9, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Sep 9, 2026
@Hweinstock
Hweinstock force-pushed the test/dedupe-project-test-helpers branch from 4f64b0a to a11d91b Compare September 9, 2026 18:57
@github-actions github-actions Bot added size/xl PR size: XL and removed size/xl PR size: XL labels Sep 9, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot added claude-security-reviewing Claude Code /security-review in progress and removed claude-security-reviewing Claude Code /security-review in progress labels Sep 9, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Sep 9, 2026
@Hweinstock
Hweinstock marked this pull request as ready for review September 9, 2026 19:03
@Hweinstock
Hweinstock force-pushed the test/dedupe-project-test-helpers branch from a11d91b to 2e2c8d4 Compare September 9, 2026 19:21
@github-actions github-actions Bot added size/xl PR size: XL and removed size/xl PR size: XL labels Sep 9, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Sep 9, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xl PR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants