Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
myitcv opened this issue Mar 19, 2020 · 11 comments
Open

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

myitcv opened this issue Mar 19, 2020 · 11 comments
Labels
enhancement New feature or request

Comments

@myitcv
Copy link
Collaborator

myitcv commented Mar 19, 2020

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

@myitcv myitcv added the enhancement New feature or request label Mar 19, 2020
@mvdan
Copy link
Collaborator

mvdan commented Apr 5, 2020

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
Copy link
Collaborator

mvdan commented Apr 5, 2020

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

@myitcv
Copy link
Collaborator Author

myitcv commented Apr 7, 2020

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
Copy link
Contributor

twpayne commented May 14, 2020

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

@myitcv
Copy link
Collaborator Author

myitcv commented May 15, 2020

@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
Copy link
Contributor

twpayne commented May 15, 2020

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
Copy link
Contributor

twpayne commented Jul 18, 2023

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
Copy link
Contributor

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
Copy link

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

cmd || true

would be nice and solve this in more general way?

@twpayne
Copy link
Contributor

twpayne commented Jul 18, 2023

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
Copy link
Collaborator

mvdan commented Sep 20, 2023

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
		},
[...]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants