Comments (6)
I think tt.assertRegexpCompileError(t, err)
needlessly increases the complexity of the test code to begin with. TBH, I had to read the code a bunch to understand it. The solution I would have done is tests := []struct { shouldFail bool
and then
re, err := regexp.Compile(tt.regexp)
if tt.shouldFail {
require.Error(t, err)
return
}
require.NoError(t, err)
match := re.MatchString("some string")
require.True(t, match)
Using a shouldFail boolean is much simpler and easier to read than having a complicated helper closure. There are times when using a closure can clean up some gnarly testing code, but checking if err should be nil or non-nil isn't one of them.
from go.
The idea that
re, err := regexp.Compile(tt.regexp)
tt.assertRegexpCompileError(t, err)
match := re.MatchString("some string")
require.True(t, match)
might stop the test with a PASS result during assertRegexpCompileError is surprising at the least. You always have to think maybe code won't continue executing due to a panic or other kind of failure, but a success?, that's something special.
The idea that an explicit return is worse because it "increases the number of possible code paths (cyclomatic complexity)" seems backward to me. The code path exists either way. The return makes it visible. That's better.
We're very unlikely to encourage this kind of hidden successful control flow by adding t.PassNow.
If you really insisted, you could always define something like
package passnow
var errPassNow = errors.New("pass now!")
func AllowPassNow() {
if e := recover(); e != nil && e != ErrPassNow {
panic(e)
}
}
func PassNow() {
panic(errPassNow)
}
and then write the tests like
t.Run(tt.name, func(t *testing.T) {
defer passnow.AllowPassNow()
re, err := regexp.Compile(tt.regexp)
tt.assertRegexpCompileError(t, err)
match := re.MatchString("some string")
require.True(t, match)
})
where calling passnow.PassNow() will panic out to the recover in AllowPassNow.
Then at least each test depending on this hidden control flow is explicitly opting in.
from go.
Similar Issues
- proposal: testing: verify failure (rather than skip) #28488
- proposal: testing: add conditional-error helpers to T and B #45650
- proposal: provide TestResult.Test function, analogous to BenchmarkResult.Benchmark #25889
- Proposal: testing.MustRun #22353
- proposal: testing: Collect test failure logs on the testing.common struct and expose collected logs via a new method on testing.TB #61860
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
from go.
re, err := regexp.Compile(tt.regexp)
tt.assertRegexpCompileError(t, err)
if err != nil {
return
}
from go.
@earthboundkid I mentioned return
in the description as a potential alternative already.
But as described, this always adds additional conditions, which increases the number of possible code paths (cyclomatic complexity) and with this, it basically raises the question "who tests the test code". It makes the test code needlessly harder to reason about (of course not a single if err != nil
, real world cases I observed can be way more involved). Therefore I think it is advisable to keep the cyclomatic complexity of the test code it self to 1.
from go.
@rsc thanks for your proposed passnow
package. I will consider this idea. I guess it boils down to unsafe
versus panic
.
I understand, that there is very little to no interest to explore this idea so I will save the time to try to counter the arguments.
It is sad, but I guess, I have to accept this.
from go.
Related Issues (20)
- proposal: cmd/build: support for using go object files when no package is provided HOT 7
- proposal: slices: add AllMatch HOT 10
- x/pkgsite: -gorepo flag does not work HOT 7
- proposal: Pipeline Operator for Go HOT 3
- -
- -
- POST->OST,GET->ET??? HOT 2
- x/pkgsite: `X` is always visible on homepage search HOT 2
- proposal: database/sql: make free connection strategy configurable HOT 4
- x/time/rate: unexpected behaviour with 0 limit HOT 2
- cmd/trace/v2: pprof profiles always empty HOT 11
- net/http: Support Transfer-Encoding: gzip HOT 2
- doc: write release notes for Go 1.24 HOT 4
- cmd/trace/v2: pprof profiles always empty [1.22 backport] HOT 4
- x/pkgsite: HOT 1
- cmd/internal/testdir: test fails when GOAMD64 is specified in goenv file HOT 2
- x/tools/gopls: evaluation of build tags used in imported packages must be optional HOT 3
- build: cmd/internal/moddeps failures HOT 1
- build: cmd/cgo/internal/testplugin failures HOT 1
- build failure >= 1.18 on windows arm32 HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from go.