Skip to content

Improve test quality in compiler_file_size_test.go - #52129

Merged
pelikhan merged 6 commits into
mainfrom
copilot/testify-expert-improve-test-quality-again
Aug 11, 2026
Merged

Improve test quality in compiler_file_size_test.go#52129
pelikhan merged 6 commits into
mainfrom
copilot/testify-expert-improve-test-quality-again

Conversation

Copilot AI commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

pkg/workflow/compiler_file_size_test.go used manual t.Errorf/t.Fatal checks, buried the MaxLockFileSize constant assertion inside an unrelated subtest, and only exercised the size-warning path with a synthetic file rather than the real code path.

Testify migration

  • Replaced manual t.Errorf/t.Fatal checks with testify assert/require.

New/reorganized coverage

  • Extracted the MaxLockFileSize == 512000 check into a standalone TestMaxLockFileSizeConstant test.
  • Added a real warning test that calls writeWorkflowOutput directly with oversized content and captures stderr to assert the actual warning message is emitted (via a new captureStderr helper, consistent with the pattern in compiler_experiments_sparse_interaction_warning_test.go).
  • Added a table-driven boundary test covering exactly MaxLockFileSize and one byte under, asserting no warning fires.
  • Added a noEmit mode test verifying no lock file is written and size validation is skipped.

Cleanup/readability

  • Renamed the synthetic lock file variable and added a comment clarifying it's not produced by an actual compile.
  • Switched to t.Cleanup for mock file removal so it runs even on early test failure.
  • captureStderr restores os.Stderr via defer so a panic or failed require inside the captured function can't leave stderr permanently redirected for later tests.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.47 AIC · ⌖ 11.6 AIC · ⊞ 8.5K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.1 AIC · ⌖ 3.4 AIC · ⊞ 8.5K ·
Comment /souschef to run again


Requested branch update from https://github.com/github/gh-aw/actions/runs/31535473039.> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 8.59 AIC · ⌖ 5.18 AIC · ⊞ 8.5K ·

Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 14.9 AIC · ⌖ 5.12 AIC · ⊞ 8.5K ·
Comment /souschef to run again

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve test quality for compiler_file_size_test.go Improve test quality in compiler_file_size_test.go Aug 11, 2026
Copilot AI requested a review from pelikhan August 11, 2026 18:40
@pelikhan
pelikhan marked this pull request as ready for review August 11, 2026 18:54
Copilot AI balanced review requested due to automatic review settings August 11, 2026 18:54

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

Pull request overview

Improves compiler lock-file size tests and assertion quality.

Changes:

  • Migrates assertions to Testify.
  • Adds real warning, boundary, and noEmit tests.
  • Adds reusable stderr capture support.
Show a summary per file
File Description
pkg/workflow/compiler_file_size_test.go Expands and reorganizes lock-file size validation tests.

Review details

Tip

Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread pkg/workflow/compiler_file_size_test.go Outdated
Comment on lines +83 to +84
// Ensure no stale lock file exists from a previous run/subtest.
_ = os.Remove(lockFile)
Comment thread pkg/workflow/compiler_file_size_test.go
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please refresh this branch, verify checks, and use the pr-finisher skill before handing back.

Unresolved review threads to address (newest first):

No failed checks were reported in the compact candidate set for this run.

Branch update was requested by sous-chef in run https://github.com/github/gh-aw/actions/runs/31527012599.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.47 AIC · ⌖ 11.6 AIC · ⊞ 8.5K ·
Comment /souschef to run again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please refresh this branch, verify checks, and use the pr-finisher skill before handing back.

Unresolved review threads to address (newest first):...

Refreshed from main, addressed both findings, and pushed 890662e. The final local validation gate passed; the referenced Sous Chef run was successful, while the current PR check was in progress when inspected.

Copilot AI requested a review from gh-aw-bot August 11, 2026 19:42
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please refresh this branch, verify checks, and use the pr-finisher skill before handing back.

Unresolved review threads to address (newest first):

No failed checks were reported in the compact candidate set for this run.

Branch update was requested by sous-chef in run https://github.com/github/gh-aw/actions/runs/31531364207.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.1 AIC · ⌖ 3.4 AIC · ⊞ 8.5K ·
Comment /souschef to run again

…t-improve-test-quality-again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please refresh this branch, verify checks, and use the pr-finisher skill before handing back.

Unresolved review threads to address (newest first):...

Refreshed and merged current main at 0221418. make agent-report-progress passed; the referenced Sous Chef run succeeded. CWI is action_required with zero jobs (no failing log available), and CI on this updated head needs maintainer re-trigger/approval.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please refresh this branch, verify checks, and use the pr-finisher skill before handing back.

No failed checks were reported in the compact candidate set for this run.

Branch update was requested by sous-chef in run https://github.com/github/gh-aw/actions/runs/31535473039.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 8.59 AIC · ⌖ 5.18 AIC · ⊞ 8.5K ·
Comment /souschef to run again

@github-actions

Copy link
Copy Markdown
Contributor

👍 Great work on this test quality improvement! The testify migration and enhanced coverage are spot-on:

  • Testify assertions — cleaner and more expressive than manual t.Errorf/t.Fatal
  • Better coverage — standalone TestMaxLockFileSizeConstant, real stderr capture tests, and boundary testing are all meaningful improvements
  • Solid stderr helpercaptureStderr is robust (restores stderr via defer even on panic) and consistent with existing patterns
  • Well-organized — the refactored subtests are easier to follow and maintain

The PR is ready for review. Nice attention to detail with the comments and cleanup patterns (e.g., t.Cleanup for safe mock teardown).

Generated by ✅ Contribution Check · auto · 60.4 AIC · ⌖ 3.55 AIC · ⊞ 8.8K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot This PR looks ready for another finishing pass. Please refresh the branch context, address any remaining reviewer feedback, and run the pr-finisher skill before handing back to maintainers.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 14.9 AIC · ⌖ 5.12 AIC · ⊞ 8.5K ·
Comment /souschef to run again

@pelikhan
pelikhan merged commit 514f144 into main Aug 11, 2026
24 of 25 checks passed
@pelikhan
pelikhan deleted the copilot/testify-expert-improve-test-quality-again branch August 11, 2026 22:51
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.

[testify-expert] Improve Test Quality: pkg/workflow/compiler_file_size_test.go

4 participants