Lint rule for making new contexts in tests (#1199)

## Description
Create a lint rule that warns if `context.TODO` or `context.Background` are called in tests. This helps standardize the use of `tester.NewContext` and ensures log messages are properly flushed during tests.

Output from the linter is similar to (sadly can't get rid of the `\\` in there)
```
pkg/services/m365/m365_test.go:49:22: use of `context.Background` forbidden because "tests should use tester\\.NewContext" (forbidigo) 
        ids, err := UserIDs(context.Background(), acct)
```

## Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [x] 💻 CI/Deployment
- [ ] 🐹 Trivial/Minor

## Issue(s)

* closes #1198 

## Test Plan

<!-- How will this be tested prior to merging.-->
- [x] 💪 Manual
- [ ]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2022-10-19 10:30:28 -07:00 committed by GitHub
parent 43a3dcbc84
commit 86740f1457
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 40 additions and 12 deletions

View File

@ -7,6 +7,7 @@ linters:
- gofmt
- gofumpt
- errcheck
- forbidigo
- lll
- misspell
- revive
@ -19,6 +20,11 @@ linters-settings:
- default
- prefix(github.com/alcionai/corso)
skip-generated: true
forbidigo:
forbid:
# Don't allow creating contexts without logging in tests. Use an ignore
# lower down to ensure usages of this outside of tests aren't reported.
- 'context\.(Background|TODO)(# tests should use tester\.NewContext )?'
lll:
line-length: 120
revive:
@ -62,21 +68,31 @@ issues:
max-same-issues: 50
exclude-rules:
- linters:
- revive
- revive
text: "exported:.*details.DetailsModel by other packages, and that stutters"
- linters:
- revive
- revive
text: "exported:.*details.DetailsEntry by other packages, and that stutters"
- linters:
- revive
- revive
text: "exported:.*mock.MockModelStore by other packages, and that stutters"
- linters:
- revive
- revive
text: "unexported-return:.*unexported type selectors.exchangeCategory"
- linters:
- revive
- revive
text: "unexported-return:.*unexported type.*kopia.conn"
- path: _test\.go
linters:
- revive
- revive
text: "import-shadowing:.*'suite' shadows"
# Don't output about context.Background or context.TODO in non-test files.
# Need complicated multi-part regex because golang doesn't support not regex
# operators for sequences of characters (only character sets). The below
# basically boils down to:
# 1. anything 4 characters long with .go suffix
# 2. anything >= 5 characters long that doesn't end in _test.go
- path: ^.{4}\.go|.*([^_].{4}|_[^t].{3}|_t[^e].{2}|_te[^s].{1}|_tes[^t])\.go
linters:
- forbidigo
text: "context.(Background|TODO)"

View File

@ -22,6 +22,7 @@ func (suite *PrintUnitSuite) TestOnly() {
t := suite.T()
c := &cobra.Command{}
// cannot use tester.NewContext() here: circular imports
//nolint:forbidigo
ctx := SetRootCmd(context.Background(), c)
assert.NoError(t, Only(ctx, nil))
assert.True(t, c.SilenceUsage)

View File

@ -181,6 +181,7 @@ func (suite *RestoreExchangeIntegrationSuite) TestExchangeRestoreCmd_badBoolFlag
}
suite.T().Run(set.String(), func(t *testing.T) {
//nolint:forbidigo
ctx := config.SetViper(context.Background(), suite.vpr)
ctx, flush := tester.WithContext(ctx)
defer flush()

View File

@ -1,7 +1,6 @@
package onedrive
import (
"context"
"strings"
"testing"
@ -35,8 +34,10 @@ func (suite *OneDriveSuite) SetupSuite() {
}
func (suite *OneDriveSuite) TestCreateGetDeleteFolder() {
ctx, flush := tester.NewContext()
defer flush()
t := suite.T()
ctx := context.Background()
folderIDs := []string{}
folderName1 := "Corso_Folder_Test_" + common.FormatNow(common.SimpleTimeTesting)
folderElements := []string{folderName1}

View File

@ -151,7 +151,9 @@ func (suite *WrapperIntegrationSuite) TestCloseAfterWrap() {
}
func (suite *WrapperIntegrationSuite) TestOpenAfterClose() {
ctx := context.Background()
ctx, flush := tester.NewContext()
defer flush()
t := suite.T()
k, err := openKopiaRepo(t, ctx)

View File

@ -1065,6 +1065,7 @@ func (suite *KopiaSimpleRepoIntegrationSuite) SetupTest() {
t := suite.T()
expectedDirs := 6
expectedFiles := len(suite.filesByPath)
//nolint:forbidigo
suite.ctx, _ = logger.SeedLevel(context.Background(), logger.Development)
c, err := openKopiaRepo(t, suite.ctx)
require.NoError(t, err)

View File

@ -36,6 +36,7 @@ func (suite *ObserveProgressUnitSuite) TestDoesThings() {
defer func() {
// don't cross-contaminate other tests.
observe.Complete()
//nolint:forbidigo
observe.SeedWriter(context.Background(), nil)
}()

View File

@ -1,7 +1,6 @@
package m365
import (
"context"
"testing"
"github.com/stretchr/testify/require"
@ -44,9 +43,12 @@ func (suite *M365IntegrationSuite) TestUsers() {
}
func (suite *M365IntegrationSuite) TestUserIDs() {
ctx, flush := tester.NewContext()
defer flush()
acct := tester.NewM365Account(suite.T())
ids, err := UserIDs(context.Background(), acct)
ids, err := UserIDs(ctx, acct)
require.NoError(suite.T(), err)
require.NotNil(suite.T(), ids)
@ -54,9 +56,12 @@ func (suite *M365IntegrationSuite) TestUserIDs() {
}
func (suite *M365IntegrationSuite) TestGetEmailAndUserID() {
ctx, flush := tester.NewContext()
defer flush()
acct := tester.NewM365Account(suite.T())
ids, err := GetEmailAndUserID(context.Background(), acct)
ids, err := GetEmailAndUserID(ctx, acct)
require.NoError(suite.T(), err)
require.NotNil(suite.T(), ids)