Skip to content
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

Use require to prevent a panic when dereferencing a nil pointer #458

Merged
merged 1 commit into from Jul 14, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 19 additions & 19 deletions admin_organization_integration_test.go
Expand Up @@ -39,7 +39,7 @@ func TestAdminOrganizations_List(t *testing.T) {
adminOrgList, err := client.Admin.Organizations.List(ctx, &AdminOrganizationListOptions{
Query: org.Name,
})
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, true, adminOrgItemsContainsName(adminOrgList.Items, org.Name))
assert.Equal(t, 1, adminOrgList.CurrentPage)
assert.Equal(t, 1, adminOrgList.TotalCount)
Expand All @@ -51,7 +51,7 @@ func TestAdminOrganizations_List(t *testing.T) {
adminOrgList, err := client.Admin.Organizations.List(ctx, &AdminOrganizationListOptions{
Query: randomName,
})
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, false, adminOrgItemsContainsName(adminOrgList.Items, org.Name))
assert.Equal(t, 1, adminOrgList.CurrentPage)
assert.Equal(t, 0, adminOrgList.TotalCount)
Expand All @@ -61,9 +61,9 @@ func TestAdminOrganizations_List(t *testing.T) {
adminOrgList, err := client.Admin.Organizations.List(ctx, &AdminOrganizationListOptions{
Include: []AdminOrgIncludeOpt{AdminOrgOwners},
})
assert.NoError(t, err)
require.NoError(t, err)

assert.NotEmpty(t, adminOrgList.Items)
require.NotEmpty(t, adminOrgList.Items)
assert.NotNil(t, adminOrgList.Items[0].Owners)
assert.NotEmpty(t, adminOrgList.Items[0].Owners[0].Email)
})
Expand Down Expand Up @@ -95,8 +95,8 @@ func TestAdminOrganizations_Read(t *testing.T) {
defer orgTestCleanup()

adminOrg, err := client.Admin.Organizations.Read(ctx, org.Name)
assert.NoError(t, err)
assert.NotNilf(t, adminOrg, "Organization is not nil")
require.NoError(t, err)
require.NotNilf(t, adminOrg, "Organization is not nil")
assert.Equal(t, adminOrg.Name, org.Name)

// attributes part of an AdminOrganization response that are not null
Expand Down Expand Up @@ -133,12 +133,12 @@ func TestAdminOrganizations_Delete(t *testing.T) {
originalOrg, _ := createOrganization(t, client)

adminOrg, err := client.Admin.Organizations.Read(ctx, originalOrg.Name)
assert.NoError(t, err)
assert.NotNilf(t, adminOrg, "Organization is not nil")
require.NoError(t, err)
require.NotNil(t, adminOrg)
assert.Equal(t, adminOrg.Name, originalOrg.Name)

err = client.Admin.Organizations.Delete(ctx, adminOrg.Name)
assert.NoError(t, err)
require.NoError(t, err)

// Cannot find deleted org
_, err = client.Admin.Organizations.Read(ctx, originalOrg.Name)
Expand Down Expand Up @@ -169,7 +169,7 @@ func TestAdminOrganizations_ModuleConsumers(t *testing.T) {
defer org2TestCleanup()

err := client.Admin.Organizations.UpdateModuleConsumers(ctx, org1.Name, []string{org2.Name})
assert.NoError(t, err)
require.NoError(t, err)

adminModuleConsumerList, err := client.Admin.Organizations.ListModuleConsumers(ctx, org1.Name, nil)
require.NoError(t, err)
Expand All @@ -181,7 +181,7 @@ func TestAdminOrganizations_ModuleConsumers(t *testing.T) {
defer org3TestCleanup()

err = client.Admin.Organizations.UpdateModuleConsumers(ctx, org1.Name, []string{org3.Name})
assert.NoError(t, err)
require.NoError(t, err)

adminModuleConsumerList, err = client.Admin.Organizations.ListModuleConsumers(ctx, org1.Name, nil)
require.NoError(t, err)
Expand Down Expand Up @@ -215,8 +215,8 @@ func TestAdminOrganizations_Update(t *testing.T) {
defer orgTestCleanup()

adminOrg, err := client.Admin.Organizations.Read(ctx, org.Name)
assert.NoError(t, err)
assert.NotNilf(t, adminOrg, "Org returned as nil")
require.NoError(t, err)
require.NotNilf(t, adminOrg, "Org returned as nil")

accessBetaTools := true
globalModuleSharing := false
Expand All @@ -235,8 +235,8 @@ func TestAdminOrganizations_Update(t *testing.T) {
}

adminOrg, err = client.Admin.Organizations.Update(ctx, org.Name, opts)
assert.NotNilf(t, adminOrg, "Org returned as nil when it shouldn't be.")
assert.NoError(t, err)
require.NotNilf(t, adminOrg, "Org returned as nil when it shouldn't be.")
require.NoError(t, err)

assert.Equal(t, accessBetaTools, adminOrg.AccessBetaTools)
assert.Equal(t, adminOrg.GlobalModuleSharing, &globalModuleSharing)
Expand All @@ -256,8 +256,8 @@ func TestAdminOrganizations_Update(t *testing.T) {
}

adminOrg, err = client.Admin.Organizations.Update(ctx, org.Name, opts)
assert.NoError(t, err)
assert.NotNilf(t, adminOrg, "Org returned as nil when it shouldn't be.")
require.NoError(t, err)
require.NotNilf(t, adminOrg, "Org returned as nil when it shouldn't be.")

assert.Equal(t, adminOrg.GlobalModuleSharing, &globalModuleSharing)
assert.Equal(t, adminOrg.IsDisabled, isDisabled)
Expand All @@ -273,8 +273,8 @@ func TestAdminOrganizations_Update(t *testing.T) {
}

adminOrg, err = client.Admin.Organizations.Update(ctx, org.Name, opts)
assert.NoError(t, err)
assert.NotNilf(t, adminOrg, "Org returned as nil when it shouldn't be.")
require.NoError(t, err)
require.NotNilf(t, adminOrg, "Org returned as nil when it shouldn't be.")

assert.Equal(t, &globalModuleSharing, adminOrg.GlobalModuleSharing)
assert.Equal(t, adminOrg.IsDisabled, isDisabled)
Expand Down
47 changes: 26 additions & 21 deletions admin_run_integration_test.go
Expand Up @@ -37,7 +37,7 @@ func TestAdminRuns_List(t *testing.T) {
rl, err := client.Admin.Runs.List(ctx, nil)
require.NoError(t, err)

assert.NotEmpty(t, rl.Items)
require.NotEmpty(t, rl.Items)
assert.Equal(t, adminRunItemsContainsID(rl.Items, rTest1.ID), true)
assert.Equal(t, adminRunItemsContainsID(rl.Items, rTest2.ID), true)
})
Expand All @@ -61,7 +61,7 @@ func TestAdminRuns_List(t *testing.T) {
},
})
require.NoError(t, err)
assert.NotEmpty(t, rl.Items)
require.NotEmpty(t, rl.Items)
assert.Equal(t, 1, rl.CurrentPage)
assert.Equal(t, adminRunItemsContainsID(rl.Items, rTest1.ID), true)
assert.Equal(t, adminRunItemsContainsID(rl.Items, rTest2.ID), true)
Expand All @@ -71,11 +71,10 @@ func TestAdminRuns_List(t *testing.T) {
rl, err := client.Admin.Runs.List(ctx, &AdminRunsListOptions{
Include: []AdminRunIncludeOpt{AdminRunWorkspace},
})
require.NoError(t, err)

assert.NoError(t, err)

assert.NotEmpty(t, rl.Items)
assert.NotNil(t, rl.Items[0].Workspace)
require.NotEmpty(t, rl.Items)
require.NotNil(t, rl.Items[0].Workspace)
Copy link
Contributor

@Uk1288 Uk1288 Jul 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the require be added to lines#76 require.NotEmpty instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! It should be thanks 👍

assert.NotEmpty(t, rl.Items[0].Workspace.Name)
})

Expand All @@ -84,11 +83,11 @@ func TestAdminRuns_List(t *testing.T) {
Include: []AdminRunIncludeOpt{AdminRunWorkspaceOrg},
})

assert.NoError(t, err)
require.NoError(t, err)
require.NotEmpty(t, rl.Items)

assert.NotEmpty(t, rl.Items)
assert.NotNil(t, rl.Items[0].Workspace)
assert.NotNil(t, rl.Items[0].Workspace.Organization)
require.NotNil(t, rl.Items[0].Workspace)
require.NotNil(t, rl.Items[0].Workspace.Organization)
assert.NotEmpty(t, rl.Items[0].Workspace.Organization.Name)
})

Expand All @@ -102,16 +101,16 @@ func TestAdminRuns_List(t *testing.T) {

t.Run("with RunStatus.pending filter", func(t *testing.T) {
r1, err := client.Runs.Read(ctx, rTest1.ID)
assert.NoError(t, err)
require.NoError(t, err)
r2, err := client.Runs.Read(ctx, rTest2.ID)
assert.NoError(t, err)
require.NoError(t, err)

// There should be pending Runs
rl, err := client.Admin.Runs.List(ctx, &AdminRunsListOptions{
RunStatus: string(RunPending),
})
assert.NoError(t, err)
assert.NotEmpty(t, rl.Items)
require.NoError(t, err)
require.NotEmpty(t, rl.Items)

assert.Equal(t, r1.Status, RunPlanning)
assert.Equal(t, adminRunItemsContainsID(rl.Items, r1.ID), false)
Expand All @@ -124,26 +123,26 @@ func TestAdminRuns_List(t *testing.T) {
rl, err := client.Admin.Runs.List(ctx, &AdminRunsListOptions{
RunStatus: string(RunApplied),
})
assert.NoError(t, err)
require.NoError(t, err)
assert.Empty(t, rl.Items)
})

t.Run("with query", func(t *testing.T) {
rl, err := client.Admin.Runs.List(ctx, &AdminRunsListOptions{
Query: rTest1.ID,
})
assert.NoError(t, err)
require.NoError(t, err)

assert.NotEmpty(t, rl.Items)
require.NotEmpty(t, rl.Items)
assert.Equal(t, adminRunItemsContainsID(rl.Items, rTest1.ID), true)
assert.Equal(t, adminRunItemsContainsID(rl.Items, rTest2.ID), false)

rl, err = client.Admin.Runs.List(ctx, &AdminRunsListOptions{
Query: rTest2.ID,
})
assert.NoError(t, err)
require.NoError(t, err)

assert.NotEmpty(t, rl.Items)
require.NotEmpty(t, rl.Items)
assert.Equal(t, adminRunItemsContainsID(rl.Items, rTest1.ID), false)
assert.Equal(t, adminRunItemsContainsID(rl.Items, rTest2.ID), true)
})
Expand Down Expand Up @@ -191,12 +190,18 @@ func TestAdminRuns_ForceCancel(t *testing.T) {
rTestPlanning, err := client.Runs.Read(ctx, rTest1.ID)
require.NoError(t, err)
assert.Equal(t, RunPlanning, rTestPlanning.Status)

require.NotNil(t, rTestPlanning.Actions)
require.NotNil(t, rTestPlanning.Permissions)
assert.Equal(t, true, rTestPlanning.Actions.IsCancelable)
assert.Equal(t, true, rTestPlanning.Permissions.CanForceCancel)

rTestPending, err := client.Runs.Read(ctx, rTest2.ID)
require.NoError(t, err)
assert.Equal(t, RunPending, rTestPending.Status)

require.NotNil(t, rTestPlanning.Actions)
require.NotNil(t, rTestPlanning.Permissions)
assert.Equal(t, true, rTestPending.Actions.IsCancelable)
assert.Equal(t, true, rTestPending.Permissions.CanForceCancel)

Expand Down Expand Up @@ -231,7 +236,7 @@ func TestAdminRuns_AdminRunsListOptions_valid(t *testing.T) {
}

err := opts.valid()
assert.NoError(t, err)
require.NoError(t, err)
})

t.Run("has invalid status", func(t *testing.T) {
Expand Down Expand Up @@ -259,7 +264,7 @@ func TestAdminRuns_AdminRunsListOptions_valid(t *testing.T) {
}

err := opts.valid()
assert.NoError(t, err)
require.NoError(t, err)
})
}

Expand Down
14 changes: 7 additions & 7 deletions admin_user_integration_test.go
Expand Up @@ -18,7 +18,7 @@ func TestAdminUsers_List(t *testing.T) {
ctx := context.Background()

currentUser, err := client.Users.ReadCurrent(ctx)
assert.NoError(t, err)
require.NoError(t, err)

org, orgCleanup := createOrganization(t, client)
defer orgCleanup()
Expand Down Expand Up @@ -79,10 +79,10 @@ func TestAdminUsers_List(t *testing.T) {
Include: []AdminUserIncludeOpt{AdminUserOrgs},
})

assert.NoError(t, err)
require.NoError(t, err)
require.NotEmpty(t, ul.Items)
require.NotEmpty(t, ul.Items[0].Organizations)

assert.NotEmpty(t, ul.Items)
assert.NotNil(t, ul.Items[0].Organizations)
assert.NotEmpty(t, ul.Items[0].Organizations[0].Name)
})

Expand All @@ -91,9 +91,9 @@ func TestAdminUsers_List(t *testing.T) {
Administrators: "true",
})

assert.NoError(t, err)
assert.NotEmpty(t, ul.Items)
assert.NotNil(t, ul.Items[0])
require.NoError(t, err)
require.NotEmpty(t, ul.Items)
require.NotNil(t, ul.Items[0])
// We use this `includesEmail` helper function because throughout
// the tests, there could be multiple admins, depending on the
// ordering of the test runs.
Expand Down
24 changes: 12 additions & 12 deletions admin_workspace_integration_test.go
Expand Up @@ -91,9 +91,9 @@ func TestAdminWorkspaces_List(t *testing.T) {
Include: []AdminWorkspaceIncludeOpt{AdminWorkspaceOrg},
})

assert.NoError(t, err)
assert.NotEmpty(t, wl.Items)
assert.NotNil(t, wl.Items[0].Organization)
require.NoError(t, err)
require.NotEmpty(t, wl.Items)
require.NotNil(t, wl.Items[0].Organization)
assert.NotEmpty(t, wl.Items[0].Organization.Name)
})

Expand All @@ -106,16 +106,16 @@ func TestAdminWorkspaces_List(t *testing.T) {
Workspace: wTest1,
}
run, err := client.Runs.Create(ctx, runOpts)
assert.NoError(t, err)
require.NoError(t, err)

wl, err := client.Admin.Workspaces.List(ctx, &AdminWorkspaceListOptions{
Include: []AdminWorkspaceIncludeOpt{AdminWorkspaceCurrentRun},
})

assert.NoError(t, err)
require.NoError(t, err)

assert.NotEmpty(t, wl.Items)
assert.NotNil(t, wl.Items[0].CurrentRun)
require.NotEmpty(t, wl.Items)
require.NotNil(t, wl.Items[0].CurrentRun)
assert.Equal(t, wl.Items[0].CurrentRun.ID, run.ID)
})
}
Expand Down Expand Up @@ -149,8 +149,8 @@ func TestAdminWorkspaces_Read(t *testing.T) {
defer workspaceCleanup()

adminWorkspace, err := client.Admin.Workspaces.Read(ctx, workspace.ID)
assert.NoError(t, err)
assert.NotNilf(t, adminWorkspace, "Admin Workspace is not nil")
require.NoError(t, err)
require.NotNilf(t, adminWorkspace, "Admin Workspace is not nil")
assert.Equal(t, adminWorkspace.ID, workspace.ID)
assert.Equal(t, adminWorkspace.Name, workspace.Name)
assert.Equal(t, adminWorkspace.Locked, workspace.Locked)
Expand Down Expand Up @@ -183,12 +183,12 @@ func TestAdminWorkspaces_Delete(t *testing.T) {
workspace, _ := createWorkspace(t, client, org)

adminWorkspace, err := client.Admin.Workspaces.Read(ctx, workspace.ID)
assert.NoError(t, err)
assert.NotNilf(t, adminWorkspace, "Admin Workspace is not nil")
require.NoError(t, err)
require.NotNilf(t, adminWorkspace, "Admin Workspace is not nil")
assert.Equal(t, adminWorkspace.ID, workspace.ID)

err = client.Admin.Workspaces.Delete(ctx, adminWorkspace.ID)
assert.NoError(t, err)
require.NoError(t, err)

// Cannot find deleted workspace
_, err = client.Admin.Workspaces.Read(ctx, workspace.ID)
Expand Down
4 changes: 3 additions & 1 deletion agent_pool_integration_test.go
Expand Up @@ -44,7 +44,9 @@ func TestAgentPoolsList(t *testing.T) {
Include: []AgentPoolIncludeOpt{AgentPoolWorkspaces},
})
require.NoError(t, err)
assert.NotEmpty(t, k.Items[0].Workspaces[0])
require.NotEmpty(t, k.Items)
require.NotEmpty(t, k.Items[0].Workspaces)
assert.NotNil(t, k.Items[0].Workspaces[0])
})

t.Run("with list options", func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions agent_token_integration_test.go
Expand Up @@ -98,7 +98,7 @@ func TestAgentTokensRead(t *testing.T) {

t.Run("read token with valid token ID", func(t *testing.T) {
at, err := client.AgentTokens.Read(ctx, token.ID)
assert.NoError(t, err)
require.NoError(t, err)
// The initial API call to create a token will return a value in the token
// object. Empty that out for comparison
token.Token = ""
Expand All @@ -125,7 +125,7 @@ func TestAgentTokensDelete(t *testing.T) {

t.Run("with valid token ID", func(t *testing.T) {
err := client.AgentTokens.Delete(ctx, token.ID)
assert.NoError(t, err)
require.NoError(t, err)
})

t.Run("without valid token ID", func(t *testing.T) {
Expand Down