testscript: add a "don't care about success" operator

[myitcv]

Per https://go-review.googlesource.com/c/go/+/223746

However per @rogpeppe on Slack, this would be a backwards incompatible change because the signature of Params.Cmds would need to change.

Options:

• Add a new field
• ...

cc @rogpeppe @mvdan

[mvdan]

I have a somewhat pressing need for this - Go 1.15 (master) is making the default -h flag report an exit status of 0 instead of the old 1, so now all of my ! mytool -h lines are failing. I could use conditions and split the line into multiple, but a simpler fix would be to use ? mytool -h instead, as I only care that the flag prints what I expect it to print.

I agree that a new field, deprecating the current Cmds (but keeping it working), would probably be the safest bet. This is similar to what Robert did with https://golang.org/pkg/go/types/#Importer - see ImporterFrom.

Something he did there, which we could emulate, is to add an extra mode integer parameter, so that future knobs may be added without yet another deprecation. I guess we could do that here, and replace both neg bool and the new want simpleStatus with two bits in a new enum-like mode. Then, the signature could end up like:

type CmdMode uint32

const (
    CmdWantFailure CmdMode = 1 << iota
    CmdWantEither
)
[...]
    Cmds map[string]func(ts *TestScript, mode CmdMode, args []string)

[mvdan]

I should add - CmdWantFailure | CmdWantEither should never happen, since they are exclusive knobs.

[myitcv]

Go 1.15 (master) is making the default -h flag report an exit status of 0 instead of the old 1

I'm assuming the implication of this change for the likes of testscript and other testers has been discussed? This feels like a fairly significant change...

Agree with your suggestion re modes BTW.

[twpayne]

https://go-review.googlesource.com/c/go/+/223746 is now merged. Maybe this issue can be closed.

[myitcv]

@twpayne it's precisely because it's been merged that we need this issue 😄 - i.e. we need to "keep up to date" with the upstream.

[twpayne]

I had a look at doing this. There's some non-trivial divergence between upstream and this package now. The easiest way to resolve this is to probably copy in from upstream again. The changes are then fairly small, mainly using exported methods instead of private methods. Otherwise, you have to pick through quite a lot of code. However, this will lead to the loss of any changes made to this package.

[twpayne]

I need this again.

As an alternative to the ? operator (which, I agree, is the correct way to do it), how about adding a new verb execdontcareaboutsuccess verb, which, er, does exactly what it says on the tin, so you can write:

execdontcareaboutsuccess mytool -h

This has the advantage of being backwards-compatible, straightforward to implement, and providing the desired functionality.

[thepudds]

Hi @twpayne, could you implement this yourself as small custom command that is specific to your project?

That might then serve as data for what to do about this issue.

[ldemailly]

not sure how easy/feasible that'd be but supporting

cmd || true

would be nice and solve this in more general way?

[twpayne]

Hi @twpayne, could you implement this yourself as small custom command that is specific to your project?

I don't think so because the existing implementation of exec uses unexported functions like ts.findBackground and ts.exec. I already have a lot of custom commands.

[mvdan]

FWIW in CUE we did a variation of what @twpayne suggested - just for one command, but it's enough to get us unblocked:

func TestMain(m *testing.M) {
    os.Exit(testscript.RunMain(m, map[string]func() int{
        "cue": MainTest,
        // Until https://github.com/rogpeppe/go-internal/issues/93 is fixed,
        // or we have some other way to use "exec" without caring about success,
        // this is an easy way for us to mimic `? exec cue`.
        "cue_exitzero": func() int {
            MainTest()
            return 0
        },
[...]