8000 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 all commits
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
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
3 changes: 2 additions & 1 deletion coderd/database/dump.sql

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- It's not possible to drop enum values from enum types, so the UP has "IF NOT
-- EXISTS".
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'workspace_proxy';
5 changes: 4 additions & 1 deletion coderd/database/models.go

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

98 changes: 97 additions & 1 deletion enterprise/audit/table_internal_test.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,29 @@
package audit

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

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/tools/go/packages"

"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/util/slice"
)

// TestAuditableResources ensures that all auditable resources are included in
// the Auditable interface and vice versa.
//
//nolint:tparallel
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 +45,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 +62,90 @@ 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
_ = 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
_ = 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
_ = audit.ResourceTarget[database.APIKey]
testAuditFunctionWithSwitch(t, auditPkg, "ResourceTarget", expectedList)
})
}

// testAuditFunctionWithSwitch is a helper function to test that a function has
// a typed switch statement that includes all the types in expectedTypes.
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
}

// findCaseTypes returns all case types in a typed switch statement. Excluding
// the "Default:" case.
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