Skip to content

test(constants): table-driven assertions and extend octal-literal check to cmd/#44114

Merged
pelikhan merged 2 commits into
mainfrom
copilot/testify-expert-improve-test-quality-again
Jul 8, 2026
Merged

test(constants): table-driven assertions and extend octal-literal check to cmd/#44114
pelikhan merged 2 commits into
mainfrom
copilot/testify-expert-improve-test-quality-again

Conversation

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

permissions_policy_test.go used raw if/t.Fatalf guards inconsistent with sibling tests, and the raw-octal AST walker only covered pkg/, leaving cmd/ unprotected.

Changes

  • Table-driven TestPermissionConstantsValues — replaces five if/t.Fatalf blocks with a []struct{name, got, want fs.FileMode} table + assert.Equal; the typed struct field also gives compile-time verification that constants are fs.FileMode (not untyped int)
  • Extend octal walker to cmd/TestNoRawOctalPermissionLiteralsInOSCalls now iterates over {pkg/, cmd/} roots so raw permission literals in cmd/ are caught
  • Testify imports — adds assert/require to align with spec_test.go; replaces the runtime.Caller guard with require.True
// Before: five sequential if/t.Fatalf blocks
if FilePermSensitive != 0o600 {
    t.Fatalf("FilePermSensitive = %v, want 0o600", FilePermSensitive)
}

// After: table-driven with testify
tests := []struct {
    name string
    got  fs.FileMode
    want fs.FileMode
}{
    {name: "FilePermSensitive", got: FilePermSensitive, want: 0o600},
    ...
}
for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
        assert.Equal(t, tt.want, tt.got)
    })
}

Generated by 👨‍🍳 PR Sous Chef · 7.79 AIC · ⌖ 6.55 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · 8.05 AIC · ⌖ 8.96 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · 9.66 AIC · ⌖ 5.93 AIC · ⊞ 7.1K ·
Comment /souschef to run again


pr-sous-chef run: https://github.com/github/gh-aw/actions/runs/28907709886

Generated by 👨‍🍳 PR Sous Chef · 13 AIC · ⌖ 12.3 AIC · ⊞ 7.1K ·
Comment /souschef to run again

…tions, cmd/ coverage

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve test quality in permissions_policy_test.go test(constants): table-driven assertions and extend octal-literal check to cmd/ Jul 7, 2026
Copilot AI requested a review from pelikhan July 7, 2026 20:34
@pelikhan pelikhan marked this pull request as ready for review July 7, 2026 20:41
Copilot AI review requested due to automatic review settings July 7, 2026 20:41

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

This PR improves the consistency and coverage of permission-policy tests in pkg/constants by refactoring constant assertions into a table-driven style and extending the existing AST-based “no raw octal permission literals” check to also scan cmd/.

Changes:

  • Refactors TestPermissionConstantsValues into a table-driven test using testify/assert for consistent assertions.
  • Expands TestNoRawOctalPermissionLiteralsInOSCalls to walk both pkg/ and cmd/ when checking for raw octal permission literals in relevant os.* calls.
  • Introduces testify/require for the runtime.Caller guard (matching patterns used in other tests in the repo).
Show a summary per file
File Description
pkg/constants/permissions_policy_test.go Refactors permission constant assertions and expands the repo-wide raw-octal permission literal guard to include cmd/.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Low

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, make the PR merge-ready, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · 7.79 AIC · ⌖ 6.55 AIC · ⊞ 7.1K ·
Comment /souschef to run again

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Hey @Copilot (and @pelikhan) 👋 — great cleanup of permissions_policy_test.go! The switch to table-driven assert.Equal assertions and the extension of the octal-literal AST walker to cover cmd/ are exactly the kind of incremental quality improvements that keep the test suite consistent and complete.

This looks ready for review. ✅

Generated by ✅ Contribution Check · 219.1 AIC · ⌖ 17 AIC · ⊞ 6.2K ·

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, make the PR merge-ready, and rerun checks once the branch is up to date.

...

HEAD remains b9637e9; the branch was already up to date with main, and there were no unresolved in-scope review threads to address.

Local merge-readiness validation passed: make fmt, make lint, make test-unit, make test, and make recompile.

The last known PR checks for the code/test workflows were green on this head; the remaining status is stale/agent-side only. I can’t re-run CI from the cloud agent in a way that verifies this HEAD, so a maintainer still needs to re-trigger checks before merge.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, make this PR merge-ready, address all unresolved review feedback, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · 8.05 AIC · ⌖ 8.96 AIC · ⊞ 7.1K ·
Comment /souschef to run again

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, make this PR merge-ready, address all unresolved review feedback, and rerun checks once the branch is up to date.

...

HEAD is still b9637e9; the branch is already up to date with main (ahead 2, behind 0).

I re-checked the review surface: there are no unresolved review threads or in-scope review comments to address. The remaining recent bot comments are approval/praise-only, not change requests.

I also re-ran local merge-readiness validation on this HEAD: make fmt, make lint, make test-unit, make test, and make recompile all passed.

The last known code/test checks on this HEAD are green. The remaining GitHub status is agent-side/pending automation, and I can’t re-trigger CI from the cloud agent in a way that verifies this HEAD, so a maintainer still needs to re-run checks before merge.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, make this PR merge-ready, address all unresolved review feedback, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · 9.66 AIC · ⌖ 5.93 AIC · ⊞ 7.1K ·
Comment /souschef to run again

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, make this PR merge-ready, address all unresolved review feedback, and rerun checks once the branch is up to date.

...

No code changes were needed here; HEAD is still b9637e9 and the PR is already up to date with main.

I re-checked the review surface and there are no unresolved in-scope review threads or review comments to address. I also re-ran local merge-readiness validation on this HEAD: make fmt, make lint, make test-unit, make test, and make recompile all passed.

The last code/test workflow runs for b9637e9 are green. The remaining active check is the agent-side copilot run for this comment; if a fresh CI cycle is still required before merge, a maintainer will need to trigger it manually.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

\n@copilot please run the skill, make this PR merge-ready, address all unresolved review feedback, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · 13 AIC · ⌖ 12.3 AIC · ⊞ 7.1K ·
Comment /souschef to run again

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🤖 PR Triage

Field Value
Category test
Risk 🟢 Low
Score 48/100 (impact:20 urgency:10 quality:18)
Action auto_merge

Rationale: CI passing. Test-only change (76+/68−) — converts raw if/t.Fatalf guards to table-driven tests, extends octal-literal coverage to cmd/. Zero production-code risk. Auto-merge candidate.

Run §28909358158

Generated by 🔧 PR Triage Agent · 112.1 AIC · ⌖ 6.5 AIC · ⊞ 5.4K ·

@pelikhan pelikhan merged commit 15ae8cd into main Jul 8, 2026
47 checks passed
@pelikhan pelikhan deleted the copilot/testify-expert-improve-test-quality-again branch July 8, 2026 01:46
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, make this PR merge-ready, address unresolved review feedback, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · 9.99 AIC · ⌖ 9.17 AIC · ⊞ 4.7K ·
Comment /souschef to run again

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.82.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[testify-expert] Improve Test Quality: pkg/constants/permissions_policy_test.go

4 participants