Overview
File: pkg/constants/permissions_policy_test.go
Source pair: pkg/constants/constants.go
Test count: 2 functions
LOC: 108
Strengths
TestNoRawOctalPermissionLiteralsInOSCalls is a well-designed repo-wide policy enforcement test using AST inspection — it catches accidental raw octal literals in os.* calls across the entire pkg/ tree.- Correct use of
t.Fatalf to abort early on setup failures (e.g. runtime.Caller failure, parse errors).
Prioritized Improvements
1. 🔴 Missing tests — high-value coverage gaps
The policy walker only covers pkg/ but not cmd/. If raw octal literals were introduced in cmd/, they would go undetected.
Additionally, TestPermissionConstantsValues only tests the constant values but not their fs.FileMode type — no test verifies that these constants are actually of type fs.FileMode (vs untyped int). Also missing:
- No test for newly added constants (e.g.,
DirPermExecutable if added later) — the current test fails silently if a constant is removed, because it only checks existing ones by name.
- No test that
cmd/ directory is also free of raw octal literals.
2. 🟡 Testify assertion upgrades
TestPermissionConstantsValues uses raw if/t.Fatalf guards instead of testify assertions, unlike sibling tests (spec_test.go, constants_test.go) which use assert/require. This creates an inconsistency in the package's test style.
Before / After: TestPermissionConstantsValues
Before:
func TestPermissionConstantsValues(t *testing.T) {
if FilePermSensitive != 0o600 {
t.Fatalf("FilePermSensitive = %v, want 0o600", FilePermSensitive)
}
if FilePermPublic != 0o644 {
t.Fatalf("FilePermPublic = %v, want 0o644", FilePermPublic)
}
// ...
}After (table-driven + testify):
func TestPermissionConstantsValues(t *testing.T) {
tests := []struct {
name string
got fs.FileMode
want fs.FileMode
}{
{"FilePermSensitive", FilePermSensitive, 0o600},
{"FilePermPublic", FilePermPublic, 0o644},
{"FilePermExecutable", FilePermExecutable, 0o755},
{"DirPermSensitive", DirPermSensitive, 0o750},
{"DirPermPublic", DirPermPublic, 0o755},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, tt.got)
})
}
}This surfaces which specific constant failed instead of stopping at the first failure.
3. 🟡 Extend policy walker to cmd/ directory
The walker in TestNoRawOctalPermissionLiteralsInOSCalls calls filepath.Walk(pkgRoot, ...) but omits cmd/. If cmd/ grows and uses raw file permission literals, it bypasses the check.
Before / After: walker scope
Before:
pkgRoot := filepath.Join(repoRoot, "pkg")
// walks only pkg/
After:
dirs := []string{
filepath.Join(repoRoot, "pkg"),
filepath.Join(repoRoot, "cmd"),
}
for _, root := range dirs {
err := filepath.Walk(root, walker)
require.NoError(t, err, "failed to walk %s", root)
}
4. 🟢 Organization / readability
- The test file does not import testify, so it cannot use
require.NoError. The t.Fatalf pattern is acceptable for now, but a require import aligns it with the rest of the package test suite.
- Consider splitting the large inline
ast.Inspect closure into a named helper (checkFileForRawOctalPerms) for better readability and testability.
Acceptance Checklist
Generated by 🧪 Daily Testify Uber Super Expert · 45.9 AIC · ⌖ 13 AIC · ⊞ 5.1K · ◷
Overview
File:
pkg/constants/permissions_policy_test.goSource pair:
pkg/constants/constants.goTest count: 2 functions
LOC: 108
Strengths
TestNoRawOctalPermissionLiteralsInOSCallsis a well-designed repo-wide policy enforcement test using AST inspection — it catches accidental raw octal literals inos.*calls across the entirepkg/tree.t.Fatalfto abort early on setup failures (e.g.runtime.Callerfailure, parse errors).Prioritized Improvements
1. 🔴 Missing tests — high-value coverage gaps
The policy walker only covers
pkg/but notcmd/. If raw octal literals were introduced incmd/, they would go undetected.Additionally,
TestPermissionConstantsValuesonly tests the constant values but not theirfs.FileModetype — no test verifies that these constants are actually of typefs.FileMode(vs untyped int). Also missing:DirPermExecutableif added later) — the current test fails silently if a constant is removed, because it only checks existing ones by name.cmd/directory is also free of raw octal literals.2. 🟡 Testify assertion upgrades
TestPermissionConstantsValuesuses rawif/t.Fatalfguards instead of testify assertions, unlike sibling tests (spec_test.go,constants_test.go) which useassert/require. This creates an inconsistency in the package's test style.Before / After: TestPermissionConstantsValues
Before:
After (table-driven + testify):
This surfaces which specific constant failed instead of stopping at the first failure.
3. 🟡 Extend policy walker to
cmd/directoryThe walker in
TestNoRawOctalPermissionLiteralsInOSCallscallsfilepath.Walk(pkgRoot, ...)but omitscmd/. Ifcmd/grows and uses raw file permission literals, it bypasses the check.Before / After: walker scope
Before:
After:
4. 🟢 Organization / readability
require.NoError. Thet.Fatalfpattern is acceptable for now, but arequireimport aligns it with the rest of the package test suite.ast.Inspectclosure into a named helper (checkFileForRawOctalPerms) for better readability and testability.Acceptance Checklist
TestPermissionConstantsValuesrefactored to table-driven usingassert.EqualTestNoRawOctalPermissionLiteralsInOSCallsextended to also walkcmd/directoryassert/require) imports added to the filemake test-unitpasses