mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2025-07-25 04:29:40 +02:00
feat: detect incorrect integration test functions (#8352)
- I have seen multiple times where a test function tries to prepare the
testing environment more than once, this can lead to bugs and false
positives of testing code. I would attribute this to lack of
documentation on how to write integration tests.
- To detect such cases, keep track when we are in a prepared test
environment and fail when some testing code is tries to once again
prepare the test environment.
- The message is logged to the function call that is requesting to
prepare the test environment, for example: `change_default_branch_test.go:19: Cannot prepare a test environment if you are already in a test environment. This is a bug in your testing code.`
A example of what this will be able to catch, 6226f464ce
:
```go
func TestFoo(t *testing.T) {
defer PrepareTestEnv(t)()
t.Run("Bar", func(t *testing.T) {
defer PrepareTestEnv(t)() // Should very likely be PrintCurrentTest
})
}
```
```go
func TestBar(t *testing.T) {
onGiteaRun(t, func(t *testing.T, _ *url.URL) {
defer PrepareTestEnv(t)() // Already called by onGiteaRun.
})
}
```
```go
func TestFooBar(t *testing.T) {
defer PrepareTestEnv(t)() // This will be called by onGiteaRun later on and very unlikely to do this before the call to onGiteaRun.
onGiteaRun(t, func(t *testing.T, _ *url.URL) {
// [...]
})
}
```
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8352
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
parent
c57dea336c
commit
4927d4ee3d
7 changed files with 47 additions and 32 deletions
|
@ -13,6 +13,7 @@ import (
|
|||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
|
@ -312,9 +313,26 @@ func PrepareCleanPackageData(t testing.TB) {
|
|||
require.NoError(t, storage.Clean(storage.Packages))
|
||||
}
|
||||
|
||||
// inTestEnv keeps track if we are current inside a test environment, this is
|
||||
// used to detect if testing code tries to prepare a test environment more than
|
||||
// once.
|
||||
var inTestEnv atomic.Bool
|
||||
|
||||
func PrepareTestEnv(t testing.TB, skip ...int) func() {
|
||||
t.Helper()
|
||||
deferFn := PrintCurrentTest(t, util.OptionalArg(skip)+1)
|
||||
|
||||
if !inTestEnv.CompareAndSwap(false, true) {
|
||||
t.Fatal("Cannot prepare a test environment if you are already in a test environment. This is a bug in your testing code.")
|
||||
}
|
||||
|
||||
deferPrintCurrentTest := PrintCurrentTest(t, util.OptionalArg(skip)+1)
|
||||
deferFn := func() {
|
||||
deferPrintCurrentTest()
|
||||
|
||||
if !inTestEnv.CompareAndSwap(true, false) {
|
||||
t.Fatal("Tried to leave test environment, but we are no longer in a test environment. This should not happen.")
|
||||
}
|
||||
}
|
||||
|
||||
cancelProcesses(t, 30*time.Second)
|
||||
t.Cleanup(func() { cancelProcesses(t, 0) }) // cancel remaining processes in a non-blocking way
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue