10000 test: Increase test coverage on auditable resources by Emyrk · Pull Request #7038 · coder/coder · GitHub
[go: up one dir, main page]

Skip to content

test: Increase test coverage on auditable resources #7038

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
test: Increase test coverage on auditable resources
When adding a new audit resource, we also need to add it to the
function switch statements. This is a likely mistake, now a unit
test will check this for you
  • Loading branch information
Emyrk committed Apr 6, 2023
commit aeb46036b2a001bb1c2d6f807274a48470995c5e
10 changes: 8 additions & 2 deletions coderd/audit/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ func ResourceTarget[T Auditable](tgt T) string {
return ""
case database.License:
return strconv.Itoa(int(typed.ID))
case database.WorkspaceProxy:
return typed.Name
default:
panic(fmt.Sprintf("unknown resource %T", tgt))
}
Expand All @@ -103,13 +105,15 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
return typed.UserID
case database.License:
return typed.UUID
case database.WorkspaceProxy:
return typed.ID
default:
panic(fmt.Sprintf("unknown resource %T", tgt))
}
}

func ResourceType[T Auditable](tgt T) database.ResourceType {
switch any(tgt).(type) {
switch typed := any(tgt).(type) {
case database.Template:
return database.ResourceTypeTemplate
case database.TemplateVersion:
Expand All @@ -128,8 +132,10 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
return database.ResourceTypeApiKey
case database.License:
return database.ResourceTypeLicense
case database.WorkspaceProxy:
return database.ResourceTypeWorkspaceProxy
default:
panic(fmt.Sprintf("unknown resource %T", tgt))
panic(fmt.Sprintf("unknown resource %T", typed))
}
}

Expand Down
1 change: 1 addition & 0 deletions coderd/database/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

93 changes: 92 additions & 1 deletion enterprise/audit/table_internal_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
package audit

import (
"fmt"
"go/types"
"strings"
"testing"

"github.com/coder/coder/coderd/util/slice"

"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/database"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/tools/go/packages"
Expand All @@ -15,7 +22,7 @@ func TestAuditableResources(t *testing.T) {
t.Parallel()

pkgs, err := packages.Load(&packages.Config{
Mode: packages.NeedTypes,
Mode: packages.NeedTypes | packages.NeedDeps,
}, "../../coderd/audit")
require.NoError(t, err)

Expand All @@ -37,13 +44,15 @@ func TestAuditableResources(t *testing.T) {
require.True(t, ok, "expected Auditable to be a union")

found := make(map[string]bool)
expectedList := make([]string, 0)
// Now we check we have all the resources in the AuditableResources
for i := 0; i < unionType.Len(); i++ {
// All types come across like 'github.com/coder/coder/coderd/database.<type>'
typeName := unionType.Term(i).Type().String()
_, ok := AuditableResources[typeName]
assert.True(t, ok, "missing resource %q from AuditableResources", typeName)
found[typeName] = true
expectedList = append(expectedList, typeName)
}

// Also check that all resources in the table are in the union. We could
Expand All @@ -52,4 +61,86 @@ func TestAuditableResources(t *testing.T) {
_, ok := found[name]
assert.True(t, ok, "extra resource %q found in AuditableResources", name)
}

// Various functions that have switch statements to include all Auditable
// resources. Make sure we have all types supported.
// nolint:paralleltest
t.Run("ResourceID", func(t *testing.T) {
// The function being tested, provided here to make it easier to find
var _ = audit.ResourceID[database.APIKey]
testAuditFunctionWithSwitch(t, auditPkg, "ResourceID", expectedList)
})

// nolint:paralleltest
t.Run("ResourceType", func(t *testing.T) {
// The function being tested, provided here to make it easier to find
var _ = audit.ResourceType[database.APIKey]
testAuditFunctionWithSwitch(t, auditPkg, "ResourceType", expectedList)
})

// nolint:paralleltest
t.Run("ResourceTarget", func(t *testing.T) {
// The function being tested, provided here to make it easier to find
var _ = audit.ResourceTarget[database.APIKey]
testAuditFunctionWithSwitch(t, auditPkg, "ResourceTarget", expectedList)
})
}

func testAuditFunctionWithSwitch(t *testing.T, pkg *packages.Package, funcName string, expectedTypes []string) {
t.Helper()

f, ok := pkg.Types.Scope().Lookup(funcName).(*types.Func)
require.True(t, ok, fmt.Sprintf("expected %s to be a function", funcName))
switchCases := findSwitchTypes(f)
for _, expected := range expectedTypes {
if !slice.Contains(switchCases, expected) {
t.Errorf("%s switch statement is missing type %q. Include it in the switch case block", funcName, expected)
}
}
for _, sc := range switchCases {
if !slice.Contains(expectedTypes, sc) {
t.Errorf("%s switch statement has unexpected type %q. Remove it from the switch case block", funcName, sc)
}
}
}

// findSwitchTypes is a helper function to find all types a switch statement in
// the function body of f has.
func findSwitchTypes(f *types.Func) []string {
caseTypes := make([]string, 0)
switches := returnSwitchBlocks(f.Scope())
for _, sc := range switches {
scTypes := findCaseTypes(sc)
caseTypes = append(caseTypes, scTypes...)
}
return caseTypes
}

func returnSwitchBlocks(sc *types.Scope) []*types.Scope {
switches := make([]*types.Scope, 0)
for i := 0; i < sc.NumChildren(); i++ {
child := sc.Child(i)
cStr := child.String()
// This is the easiest way to tell if it is a switch statement.
if strings.Contains(cStr, "type switch scope") {
switches = append(switches, child)
}
}
return switches
}

func findCaseTypes(sc *types.Scope) []string {
caseTypes := make([]string, 0)
for i := 0; i < sc.NumChildren(); i++ {
child := sc.Child(i)
for _, name := range child.Names() {
obj := child.Lookup(name).Type()
typeName := obj.String()
// Ignore the "Default:" case
if typeName != "any" {
caseTypes = append(caseTypes, typeName)
}
}
}
return caseTypes
}
0