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.

Sign up for GitHub

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

Jump to bottom

Allow custom commands to write to stdout and stderr #139

Closed
matthewmueller opened this issue May 2, 2021 · 2 comments
Closed

Allow custom commands to write to stdout and stderr #139

matthewmueller opened this issue May 2, 2021 · 2 comments

Comments

@matthewmueller
Copy link

matthewmueller commented May 2, 2021 •
edited
Loading

Hey there, I've had a couple cases where it'd be nice to be able to write to stdout and stderr from inside a custom command.

For example:

func Test(t *testing.T) {
	p := testscript.Params{
		Cmds: map[string]func(ts *testscript.TestScript, neg bool, args []string){
			"render": render,
		},
	}
	testscript.Run(t, p)
}

func render(ts *testscript.TestScript, neg bool, args []string) {
    html, err := render.Render(ts.MkAbs(args))
    ts.Check(err)
    _ = html
}
render 'index.gohtml'

-- index.gohtml --
<html>
  <body>
    <h1>{{.}}</h1>
  </body>
</html>

Unfortunately I haven't found a way to use the command's results within the test. The workaround I use now is to write a file and then read from it.

What I'd like to be able to do is write a testscript like this:

render 'index.gohtml'
stdout '<h1>'

-- index.gohtml --
<html>
  <body>
    <h1>{{.}}</h1>
  </body>
</html>

And the failure case:

! render 'index.gohtml'
stderr 'bad HTML'

-- index.gohtml --
<html

Thanks for extracting and improving this package!
@matthewmueller matthewmueller changed the title Allow custom commands to buffer stdout or stderr Allow custom commands to write to stdout and stderr May 2, 2021
matthewmueller added a commit to matthewmueller/go-internal that referenced this issue May 2, 2021
@matthewmueller
testscript: support writing to stdio from inside a custom command. cl…
1ba84f9 
…oses rogpeppe#139.
@matthewmueller matthewmueller mentioned this issue May 2, 2021
Closed
@matthewmueller
Copy link
Author

FWIW opened a PR for this one here: #140

myitcv added a commit that referenced this issue Apr 25, 2023
@myitcv
testscript: expose (*TestScript).stdout via Stdout()
4ca5004 
Similarly, expose (*TestScript).stderr via Stderr().

Closes #139
myitcv added a commit that referenced this issue Apr 25, 2023
@myitcv
testscript: expose (*TestScript).stdout via Stdout()
c5cb324 
Similarly, expose (*TestScript).stderr via Stderr().

Closes #139
myitcv added a commit that referenced this issue Apr 26, 2023
@myitcv
testscript: expose (*TestScript).stdout via SetStdout(string)
f9546cf 
Similarly, expose (*TestScript).stderr via SetStderr(string).

Closes #139
myitcv added a commit that referenced this issue Apr 26, 2023
@myitcv
testscript: expose (*TestScript).stdout via Stdout()
bc52749 
Similarly, expose (*TestScript).stderr via Stderr().

Closes #139
@myitcv
Copy link
Collaborator

myitcv commented Apr 26, 2023

Picking up API conversation from #217 (review)

  • Please add a test case for a command that writes to stdout and stderr, and then returns an error. I wonder if stderr is then kept at all, for example.

I don't think this is actually something we can test. A Params.Cmds command has complete control on whether execution is a failure or not. The neg parameter indicates whether the user intended the command to fail or not. So the only "error" situation is when the execution of such a command does not concur with neg. In that situation, execution terminates immediately.

The real problem is the API here. We're trying to work around the fact that a Params.Cmds command should return stdout and stderr, rather than setting it via the approach proposed in #217, an approach which implies it can be done multiple times... which isn't the case. Just as such a command can only call (*TestScript).Fatalf() once.

Fixing this properly, however, would either require us to declare a new API via another field on Params, or by loosening the type of Params.Cmds to allow for functions that have the signature:

func(ts *TestScript, neg bool, args []string) (stdout, stderr string, err error)

@myitcv myitcv mentioned this issue Apr 26, 2023
Closed
myitcv added a commit that referenced this issue Apr 26, 2023
@myitcv
testscript: expose (*TestScript).stdout via Stdout()
98ec711 
Similarly, expose (*TestScript).stderr via Stderr().

Closes #139
myitcv added a commit that referenced this issue Apr 26, 2023
@myitcv
testscript: expose (*TestScript).stdout via Stdout()
cbce95d 
Similarly, expose (*TestScript).stderr via Stderr().

Closes #139
myitcv added a commit that referenced this issue Apr 26, 2023
@myitcv
testscript: expose (*TestScript).stdout via Stdout()
6d2e77c 
Similarly, expose (*TestScript).stderr via Stderr().

Closes #139
myitcv added a commit that referenced this issue Apr 26, 2023
@myitcv
testscript: expose (*TestScript).stdout via Stdout()
d3aa1fd 
Similarly, expose (*TestScript).stderr via Stderr().

Closes #139
myitcv added a commit that referenced this issue Apr 26, 2023
@myitcv
testscript: expose (*TestScript).stdout via Stdout()
ebf36fa 
Similarly, expose (*TestScript).stderr via Stderr().

Closes #139
myitcv added a commit that referenced this issue Apr 27, 2023
@myitcv
testscript: expose (*TestScript).stdout via Stdout()
ca28326 
Similarly, expose (*TestScript).stderr via Stderr().

Closes #139
@myitcv myitcv closed this as completed in 22b9127 Apr 27, 2023
ldemailly pushed a commit to fortio/testscript that referenced this issue May 7, 2023
@myitcv @ldemailly
testscript: expose (*TestScript).stdout via Stdout() (rogpeppe#216)
6e3acf8 
Similarly, expose (*TestScript).stderr via Stderr().

Closes rogpeppe#139
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

testscript: support writing to stdio from inside a custom command matthewmueller/go-internal
2 participants
@matthewmueller @myitcv