smartystreets / goconvey

Go testing in the browser. Integrates with `go test`. Write behavioral tests in Go.
http://smartystreets.github.io/goconvey/
Other
8.23k stars 554 forks source link

Support Courtney #645

Open mtiller opened 2 years ago

mtiller commented 2 years ago

It amazes me that the built in go test -cover functionality doesn't support explicit exclusion of code (e.g., error conditions that cannot be reliably triggered). Fortunately, a tool called courtney does support this using special // notest comments.

But goconvey uses the standard go test mechanism for generating coverage reports. As a result, I have to run courtney separately. This begs the question...would it be possible for goconvey to use an alternative tool chain to generate coverage? I would guess yes. If somebody could point me to the relevant code and perhaps provide a basic explanation of how it triggers go test, I could consider putting together a PR with courtney support if there is a willingness to accept such a PR.

Or perhaps there is some easy, existing way to change this via existing configuration variables?!?

riannucci commented 2 years ago

I'm actually in the process of sprucing up goconvey right now; Did you know it still has detection for Go 1.2? (not 1.12. 1.2). It also still does manual (incorrect) parsing of go test output, even though go test -json exists...

I wasn't aware of courtney but I've definitely missed the ability to mark some sections of code as uncovered.... I'll look into this. It's possible that it'll be supportable as a flag of some sort, but I want to finish modernizing goconvey a bit first.

riannucci commented 2 years ago

Note to self: https://github.com/dave/courtney

So since this accepts regular go test options under the -t flag, I think it would be possible to modularize https://github.com/smartystreets/goconvey/blob/master/web/server/system/shell.go#L31 a bit (which is initialized here: https://github.com/smartystreets/goconvey/blob/master/goconvey.go#L65).

I don't think we'd want to directly depend on courtney in go.mod though (I'm actually hoping I can reduce the dependencies there even further), but I think it should be reasonable to have a mode in goconvey which detects it.

I assume courtney still produces html reports with the same names as go test?

riannucci commented 2 years ago

maybe CLI syntax like goconvey -testRunner courtney which then invokes the courtney implementation inside of shell?

mtiller commented 2 years ago

I'd be perfectly content with just adding a flag in a .goconvey file. Totally agree on the "no extra dependencies". I don't think you need it as a dependency. Just a way to invoke alternative tools for generating the coverage data. courtney generates a .out file that you can pass to normal go tooling in order to generate HTML. For example, this is how I use it:

$ courtney -t="-count=1" -v
$ go tool cover -html=coverage.out -o coverage.html

The key point here is that courtney is just a tool for generating the coverage data, not the HTML report.

mtiller commented 2 years ago

FYI, I cloned the goconvey repo and changed the return statement from runWithCoverage to:

   return NewCommand(directory, "courtney", "-o", reportPath, "-v")

It runs and gives me output. But something odd is going on. The coverage numbers shown in the upper left corner of the web interface look like the non-courtney numbers and if I click on any of the links there to see the underlying HTML, I got a 404. But I checked and both the coverage data and HTML reports are being generated exactly where goconvey asked for them to be generated (e.g., reportPath) so I don't understand why it isn't appearing.

riannucci commented 2 years ago

There were some bugs with how goconvey was serving the HTML; I think they may be fixed since you tried this.

I'm hoping to spend some more time on goconvey + convey over the holiday break, but I don't know much time I'll actually have :)