Improve test quality in compiler_file_size_test.go - #52129
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Improves compiler lock-file size tests and assertion quality.
Changes:
- Migrates assertions to Testify.
- Adds real warning, boundary, and
noEmittests. - 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
| // Ensure no stale lock file exists from a previous run/subtest. | ||
| _ = os.Remove(lockFile) |
|
@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.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Refreshed from |
|
@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.
|
…t-improve-test-quality-again Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Refreshed and merged current |
|
@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.
|
|
👍 Great work on this test quality improvement! The testify migration and enhanced coverage are spot-on:
The PR is ready for review. Nice attention to detail with the comments and cleanup patterns (e.g.,
|
|
@copilot This PR looks ready for another finishing pass. Please refresh the branch context, address any remaining reviewer feedback, and run the
|
pkg/workflow/compiler_file_size_test.goused manualt.Errorf/t.Fatalchecks, buried theMaxLockFileSizeconstant assertion inside an unrelated subtest, and only exercised the size-warning path with a synthetic file rather than the real code path.Testify migration
t.Errorf/t.Fatalchecks withtestifyassert/require.New/reorganized coverage
MaxLockFileSize == 512000check into a standaloneTestMaxLockFileSizeConstanttest.writeWorkflowOutputdirectly with oversized content and captures stderr to assert the actual warning message is emitted (via a newcaptureStderrhelper, consistent with the pattern incompiler_experiments_sparse_interaction_warning_test.go).MaxLockFileSizeand one byte under, asserting no warning fires.noEmitmode test verifying no lock file is written and size validation is skipped.Cleanup/readability
t.Cleanupfor mock file removal so it runs even on early test failure.captureStderrrestoresos.Stderrviadeferso a panic or failedrequireinside the captured function can't leave stderr permanently redirected for later tests.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 · ◷