Skip to content

fix: Remove use of require in require.Eventually in tests #3110

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 4 commits into from
Jul 22, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,9 @@ func TestAgent(t *testing.T) {
if runtime.GOOS == "windows" {
// Windows uses UTF16! 🪟🪟🪟
content, _, err = transform.Bytes(unicode.UTF16(unicode.LittleEndian, unicode.UseBOM).NewDecoder(), content)
require.NoError(t, err)
if !assert.NoError(t, err) {
return false
}
}
gotContent = string(content)
return true
Expand Down
16 changes: 7 additions & 9 deletions cli/resetpassword_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,15 @@ func TestResetPassword(t *testing.T) {
err = serverCmd.ExecuteContext(ctx)
assert.ErrorIs(t, err, context.Canceled)
}()
var client *codersdk.Client
var rawURL string
require.Eventually(t, func() bool {
rawURL, err := cfg.URL().Read()
if err != nil {
return false
}
accessURL, err := url.Parse(rawURL)
require.NoError(t, err)
client = codersdk.New(accessURL)
return true
rawURL, err = cfg.URL().Read()
return err == nil && rawURL != ""
}, 15*time.Second, 25*time.Millisecond)
accessURL, err := url.Parse(rawURL)
require.NoError(t, err)
client := codersdk.New(accessURL)

_, err = client.CreateFirstUser(ctx, codersdk.CreateFirstUserRequest{
Email: email,
Username: username,
Expand Down
16 changes: 7 additions & 9 deletions cli/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,15 @@ func TestServer(t *testing.T) {
go func() {
errC <- root.ExecuteContext(ctx)
}()
var client *codersdk.Client
var rawURL string
require.Eventually(t, func() bool {
rawURL, err := cfg.URL().Read()
if err != nil {
return false
}
accessURL, err := url.Parse(rawURL)
assert.NoError(t, err)
client = codersdk.New(accessURL)
return true
rawURL, err = cfg.URL().Read()
return err == nil && rawURL != ""
}, time.Minute, 50*time.Millisecond)
accessURL, err := url.Parse(rawURL)
require.NoError(t, err)
client := codersdk.New(accessURL)

_, err = client.CreateFirstUser(ctx, codersdk.CreateFirstUserRequest{
Email: "some@one.com",
Username: "example",
Expand Down
3 changes: 1 addition & 2 deletions coderd/coderd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
// so we wait for it to occur.
require.Eventually(t, func() bool {
provisionerds, err := client.ProvisionerDaemons(ctx)
require.NoError(t, err)
return len(provisionerds) > 0
return assert.NoError(t, err) && len(provisionerds) > 0
}, time.Second*10, time.Second)

provisionerds, err := client.ProvisionerDaemons(ctx)
Expand Down
21 changes: 15 additions & 6 deletions coderd/coderdtest/coderdtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,8 @@ func CreateWorkspaceBuild(
t *testing.T,
client *codersdk.Client,
workspace codersdk.Workspace,
transition database.WorkspaceTransition) codersdk.WorkspaceBuild {
transition database.WorkspaceTransition,
) codersdk.WorkspaceBuild {
req := codersdk.CreateWorkspaceBuildRequest{
Transition: codersdk.WorkspaceTransition(transition),
}
Expand Down Expand Up @@ -397,36 +398,44 @@ func UpdateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID

// AwaitTemplateImportJob awaits for an import job to reach completed status.
func AwaitTemplateVersionJob(t *testing.T, client *codersdk.Client, version uuid.UUID) codersdk.TemplateVersion {
t.Helper()

t.Logf("waiting for template version job %s", version)
var templateVersion codersdk.TemplateVersion
require.Eventually(t, func() bool {
var err error
templateVersion, err = client.TemplateVersion(context.Background(), version)
require.NoError(t, err)
return templateVersion.Job.CompletedAt != nil
return assert.NoError(t, err) && templateVersion.Job.CompletedAt != nil
}, 5*time.Second, 25*time.Millisecond)
return templateVersion
}

// AwaitWorkspaceBuildJob waits for a workspace provision job to reach completed status.
func AwaitWorkspaceBuildJob(t *testing.T, client *codersdk.Client, build uuid.UUID) codersdk.WorkspaceBuild {
t.Helper()

t.Logf("waiting for workspace build job %s", build)
var workspaceBuild codersdk.WorkspaceBuild
require.Eventually(t, func() bool {
var err error
workspaceBuild, err = client.WorkspaceBuild(context.Background(), build)
require.NoError(t, err)
return workspaceBuild.Job.CompletedAt != nil
return assert.NoError(t, err) && workspaceBuild.Job.CompletedAt != nil
}, 5*time.Second, 25*time.Millisecond)
return workspaceBuild
}

// AwaitWorkspaceAgents waits for all resources with agents to be connected.
func AwaitWorkspaceAgents(t *testing.T, client *codersdk.Client, build uuid.UUID) []codersdk.WorkspaceResource {
t.Helper()

t.Logf("waiting for workspace agents (build %s)", build)
var resources []codersdk.WorkspaceResource
require.Eventually(t, func() bool {
var err error
resources, err = client.WorkspaceResourcesByBuild(context.Background(), build)
require.NoError(t, err)
if !assert.NoError(t, err) {
return false
}
for _, resource := range resources {
for _, agent := range resource.Agents {
if agent.Status != codersdk.WorkspaceAgentConnected {
Expand Down
4 changes: 3 additions & 1 deletion coderd/devtunnel/tunnel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ func TestTunnel(t *testing.T) {

require.Eventually(t, func() bool {
res, err := fTunServer.requestHTTP()
require.NoError(t, err)
if !assert.NoError(t, err) {
return false
}
defer res.Body.Close()
_, _ = io.Copy(io.Discard, res.Body)

Expand Down
4 changes: 2 additions & 2 deletions coderd/provisionerdaemons_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/coder/coder/coderd/coderdtest"
Expand Down Expand Up @@ -39,8 +40,7 @@ func TestProvisionerDaemons(t *testing.T) {
require.Eventually(t, func() bool {
var err error
version, err = client.TemplateVersion(context.Background(), version.ID)
require.NoError(t, err)
return version.Job.Error != ""
return assert.NoError(t, err) && version.Job.Error != ""
}, 5*time.Second, 25*time.Millisecond)
})
}
Expand Down
30 changes: 20 additions & 10 deletions coderd/templateversions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ func TestPatchCancelTemplateVersion(t *testing.T) {
require.Eventually(t, func() bool {
var err error
version, err = client.TemplateVersion(context.Background(), version.ID)
require.NoError(t, err)
if !assert.NoError(t, err) {
return false
}
t.Logf("Status: %s", version.Job.Status)
return version.Job.Status == codersdk.ProvisionerJobRunning
}, 5*time.Second, 25*time.Millisecond)
Expand Down Expand Up @@ -148,7 +150,9 @@ func TestPatchCancelTemplateVersion(t *testing.T) {
require.Eventually(t, func() bool {
var err error
version, err = client.TemplateVersion(context.Background(), version.ID)
require.NoError(t, err)
if !assert.NoError(t, err) {
return false
}
t.Logf("Status: %s", version.Job.Status)
return version.Job.Status == codersdk.ProvisionerJobRunning
}, 5*time.Second, 25*time.Millisecond)
Expand Down Expand Up @@ -536,9 +540,7 @@ func TestTemplateVersionDryRun(t *testing.T) {
// Wait for the job to complete
require.Eventually(t, func() bool {
job, err := client.TemplateVersionDryRun(ctx, version.ID, job.ID)
assert.NoError(t, err)

return job.Status == codersdk.ProvisionerJobSucceeded
return assert.NoError(t, err) && job.Status == codersdk.ProvisionerJobSucceeded
}, 5*time.Second, 25*time.Millisecond)

<-logsDone
Expand Down Expand Up @@ -588,7 +590,8 @@ func TestTemplateVersionDryRun(t *testing.T) {
{
Type: &proto.Provision_Response_Log{
Log: &proto.Log{},
}},
},
},
{
Type: &proto.Provision_Response_Complete{
Complete: &proto.Provision_Complete{},
Expand All @@ -609,7 +612,9 @@ func TestTemplateVersionDryRun(t *testing.T) {

require.Eventually(t, func() bool {
job, err := client.TemplateVersionDryRun(context.Background(), version.ID, job.ID)
assert.NoError(t, err)
if !assert.NoError(t, err) {
return false
}

t.Logf("Status: %s", job.Status)
return job.Status == codersdk.ProvisionerJobPending
Expand All @@ -620,7 +625,9 @@ func TestTemplateVersionDryRun(t *testing.T) {

require.Eventually(t, func() bool {
job, err := client.TemplateVersionDryRun(context.Background(), version.ID, job.ID)
assert.NoError(t, err)
if !assert.NoError(t, err) {
return false
}

t.Logf("Status: %s", job.Status)
return job.Status == codersdk.ProvisionerJobCanceling
Expand All @@ -642,7 +649,9 @@ func TestTemplateVersionDryRun(t *testing.T) {

require.Eventually(t, func() bool {
job, err := client.TemplateVersionDryRun(context.Background(), version.ID, job.ID)
assert.NoError(t, err)
if !assert.NoError(t, err) {
return false
}

t.Logf("Status: %s", job.Status)
return job.Status == codersdk.ProvisionerJobSucceeded
Expand All @@ -666,7 +675,8 @@ func TestTemplateVersionDryRun(t *testing.T) {
{
Type: &proto.Provision_Response_Log{
Log: &proto.Log{},
}},
},
},
{
Type: &proto.Provision_Response_Complete{
Complete: &proto.Provision_Complete{},
Expand Down
3 changes: 1 addition & 2 deletions coderd/workspacebuilds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,7 @@ func TestPatchCancelWorkspaceBuild(t *testing.T) {
require.Eventually(t, func() bool {
var err error
build, err = client.WorkspaceBuild(context.Background(), workspace.LatestBuild.ID)
require.NoError(t, err)
return build.Job.Status == codersdk.ProvisionerJobRunning
return assert.NoError(t, err) && build.Job.Status == codersdk.ProvisionerJobRunning
}, 5*time.Second, 25*time.Millisecond)
err := client.CancelWorkspaceBuild(context.Background(), build.ID)
require.NoError(t, err)
Expand Down
19 changes: 15 additions & 4 deletions scripts/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,17 @@ func doNotCallTFailNowInsideGoroutine(m dsl.Matcher) {
Where(m["require"].Text == "require").
Report("Do not call functions that may call t.FailNow in a goroutine, as this can cause data races (see testing.go:834)")

// require.Eventually runs the function in a goroutine.
m.Match(`
require.Eventually(t, func() bool {
$*_
$require.$_($*_)
$*_
}, $*_)`).
At(m["require"]).
Where(m["require"].Text == "require").
Report("Do not call functions that may call t.FailNow in a goroutine, as this can cause data races (see testing.go:834)")

m.Match(`
go func($*_){
$*_
Expand Down Expand Up @@ -90,10 +101,10 @@ func InTx(m dsl.Matcher) {
At(m["f"]).
Report("Do not use the database directly within the InTx closure. Use '$y' instead of '$x'.")

//When using a tx closure, ensure that if you pass the db to another
//function inside the closure, it is the tx.
//This will miss more complex cases such as passing the db as apart
//of another struct.
// When using a tx closure, ensure that if you pass the db to another
// function inside the closure, it is the tx.
// This will miss more complex cases such as passing the db as apart
// of another struct.
m.Match(`
$x.InTx(func($y database.Store) error {
$*_
Expand Down