search

gotestyourself / gotestsum

'go test' runner with output optimized for humans, JUnit XML for CI integration, and a summary of the test results.
Apache License 2.0
1.99k stars 118 forks source link

Add support for gotestsum on illumos/amd64 #373

Closed szaydel closed 9 months ago

szaydel commented 9 months ago

Please consider adding support for illumos-based operating systems such as OmniOS, OpenIndiana, SmartOS, etc. I tested to make sure things build correctly and validated that the tool works correctly on a couple of versions of OmniOS. Addition of internal/filewatcher/term_illumos.go was necessary to correctly build, and I apologize, but these constants are not available from golang.org/x/sys/unix, thus I supplied literal values instead. These values have not changed in at least 20 years, likely much longer.

szaydel commented 9 months ago

Oh, this is the ignore list! It only builds on one. This looks good to me

Yeah, that's what I was just trying to say. Thanks for taking a second look.

szaydel commented 9 months ago

Thank you again for merging this change in, I appreciate it!

szaydel commented 9 months ago

Yes, nearly zero chance of changing, but you are right using these named constants makes sense. I somehow did not see them. If they won't require a go.mod update I will happily switch to them. Magic numbers are never great. I will double check and do a PR when I have a chance. Thanks again for the suggestion!

On Wed, Oct 11, 2023, 5:27 PM Scott Moynes @.***> wrote:

@.**** commented on this pull request.

In internal/filewatcher/term_illumos.go https://github.com/gotestyourself/gotestsum/pull/373#discussion_r1355911741 :

+const tcGet = 0x540d +const tcSet = 0x540e

You are almost certainly right that these constants likely won't change ever, but I wonder if you can't use unix.TCGETS and unix.TCSETS, respectively? If nothing else, having a name makes it a bit less mysterious from where these values originate.

These constants do seem to be present in golang.org/x/sys/unix when GOOS=illumos, at least with recent versions.

$ GOOS=illumos go doc golang.org/x/sys/unix.TCGETS... TCGETS = 0x540d...

— Reply to this email directly, view it on GitHub https://github.com/gotestyourself/gotestsum/pull/373#pullrequestreview-1672787074, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB44UGBMUV3OZJUYHXSYCLX642OXANCNFSM6AAAAAA5XZXAK4 . You are receiving this because you authored the thread.Message ID: @.***>

szaydel commented 9 months ago

@smoynes, thanks again. I put up a PR with the change you recommended after making sure that things look correct. https://github.com/gotestyourself/gotestsum/pull/375