Skip to content

Conversation

@ikonst
Copy link

@ikonst ikonst commented Nov 27, 2025

Introduces (*Suite).SyncTest which is to synctest.Run like (*Suite).Run is to (*testing.T).Run.

Its role is to maintain s.T() (so it points to synctest bubble's t).

synctest was introduced in go 1.25, so this is built conditionally.

@ikonst
Copy link
Author

ikonst commented Dec 2, 2025

@brackendawson, can take a look please?

@StevenACoffman
Copy link

Hey, thanks for adding this!

@Nocccer
Copy link

Nocccer commented Dec 4, 2025

I created a issue about support of synctest in suite. Even do your change is nice, probably alot of people will use it wrong. You can't reuse channels inside the bubble, if they are created outside the bubble (e.g. if they are part of the suite).

@ikonst
Copy link
Author

ikonst commented Dec 5, 2025

I created a issue about support of synctest in suite. Even do your change is nice, probably alot of people will use it wrong. You can't reuse channels inside the bubble, if they are created outside the bubble (e.g. if they are part of the suite).

It's a fair point that we need to think of an API that makes sense for how people normally use suites. Let me think about it.

}

func TestSyncTest(t *testing.T) {
t.Setenv("GODEBUG", "asynctimerchan=0") // since our go.mod says `go 1.17`

Choose a reason for hiding this comment

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

BTW, Not sure if it's worth referencing the specific release notes https://go.dev/doc/go1.23#timer-changes

@StevenACoffman
Copy link

As an alternative experiment, I locally forked the entire suite package to make a Go 1.25 syncsuite package, and only modified the end ofsuite.go file's last two lines of func Run(t *testing.T, suite TestingSuite) { and the following function: func runTests(t *testing.T, tests []test) { as follows (signature change intended):

	runTests(t, suite, tests)
}

func runTests(t *testing.T, suite TestingSuite, tests []test) {
	if len(tests) == 0 {
		t.Log("warning: no tests to run")
		return
	}
	oldT := suite.T()
	for _, currentTest := range tests {
		synctest.Test(t, func(t *testing.T) {
			suite.SetT(t)
			defer suite.SetT(oldT)
			t.Run(currentTest.name, currentTest.run)
		})
	}
}

@Nocccer
Copy link

Nocccer commented Dec 5, 2025

As an alternative experiment, I locally forked the entire suite package to make a Go 1.25 syncsuite package, and only modified the end ofsuite.go file's last two lines of func Run(t *testing.T, suite TestingSuite) { and the following function: func runTests(t *testing.T, tests []test) { as follows (signature change intended):

	runTests(t, suite, tests)
}

func runTests(t *testing.T, suite TestingSuite, tests []test) {
	if len(tests) == 0 {
		t.Log("warning: no tests to run")
		return
	}
	oldT := suite.T()
	for _, currentTest := range tests {
		synctest.Test(t, func(t *testing.T) {
			suite.SetT(t)
			defer suite.SetT(oldT)
			t.Run(currentTest.name, currentTest.run)
		})
	}
}

Note that a suite only for synctest shall not provide SetupSuite. I don't know if inside a synctest you can use normal subtests.

Update: I also did a fast draft and added direct synctest support into the suite by prefixing synctests with SyncTest. It works in simple cases but breaks in special cases. I added tests where it works and where it fails.
master...Nocccer:testify:feature/suite-synctest

Probably the solution is a new suite that runs each test inside a bubble and does not allow to have SetupSuite and subtest functionality.

@ikonst
Copy link
Author

ikonst commented Dec 7, 2025

Forking Suite into SyncSuite without a Run or a SetupSuite makes sense to me.

Suite methods:

  • Assert
  • Require
  • T
  • Run
  • SetS
  • SetT

The first 3 methods can be moved into a common struct.

@Nocccer
Copy link

Nocccer commented Dec 7, 2025

Forking Suite into SyncSuite without a Run or a SetupSuite makes sense to me.

Suite methods:

  • Assert
  • Require
  • T
  • Run
  • SetS
  • SetT

The first 3 methods can be moved into a common struct.

I tried to create a new SyncSuite yesterday. The problem is that Suite is supported till go 1.17. We can't extend the global Run method to check what the suite embeeds (Suite or SyncSuite), because for go <1.25 it will not compile, because SyncSuite will be undefined. The only way to add a new suite type without breaking go compatibility, would be to provide another method to run a SyncSuite or create a new package.

Maybe i am dumb and it can work. I don't have experience with build flags so far. I will try out more today.

Update: Here is a branch where i added SyncTest and splitt the existing Suite into reusable parts.
https://github.com/Nocccer/testify/tree/feature/synctest-suite

@ikonst
Copy link
Author

ikonst commented Dec 8, 2025

Did you try two mutually exclusive files, one for go1.25 and one for !go1.25?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants