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

cmd/testscript: work directory printed twice #170

Open
bitfield opened this issue Jul 28, 2022 · 5 comments
Open

cmd/testscript: work directory printed twice #170

bitfield opened this issue Jul 28, 2022 · 5 comments

Comments

@bitfield
Copy link
Contributor

I noticed something unexpected when using the standalone testscript tool:

testscript -work testdata/script/echo.txtar   
temporary work directory: /var/folders/q3/ybqqxyh92yd0yc865zqk0vpm0000gn/T/testscript2448880374
temporary work directory for testdata/script/echo.txtar: /private/var/folders/q3/ybqqxyh92yd0yc865zqk0vpm0000gn/T/go-test-script652423740/script-script

It prints out two working directories, with only the first one actually being preserved. I think this is because testscript -work not only prints the working directory itself, but also creates a test runner with testWork: true, which causes the runner to print it, too. Is this intended? For a single script, I would have expected to see just one working directory.

Repro:

exec testscript -work example.txtar
stderr -count=1 'temporary work directory'

-- example.txtar --
exec echo hello
@myitcv
Copy link
Collaborator

myitcv commented Jul 29, 2022

This is one of the most wonderfully meta bug reports in the use of cmd/testscript; thanks for reporting!

The challenge for cmd/testscript is the following. cmd/testscript can take an arbitrary number of input files (or stdin). The thinking was that in case of any issues/errors in any of those scripts, that the user would expect to see file paths "similar" to those supplied as arguments to cmd/testscript. Hence, we broadly retain the dir path for each argument.

However, this leads to the next problem: testscript the library (currently) takes a Dir input. Because we can't be sure that any of the directories containing the files provided as arguments to cmd/testscript don't contain other scripts we don't want to run, we have to copy the input files to a temporary directory structure from which we "run" testscript. So this is where the first temporary directory comes from.

When it comes to running testscript on the copied input files in that temporary directory, we then get the second temporary directory you see reported.

According to my testing there is a bug however in that only the first temporary directory is retained; the second should also be retained, because that's the more interesting of the two.

Which leads to another thought: I think the printing of the first temporary directory is actually redundant. The fact we have to "swing" the input files via a temporary directory in order to run testscript on them is a detail the user doesn't really care about.

So I think I'd support a change that:

  • removes the printing of the first temporary directory;
  • unconditionally removes the first temporary directory (because it's an implementation detail);
  • fixes the fact that the testscript temporary directory is not retained.

@bitfield
Copy link
Contributor Author

Thanks for the reply! You're quite right to emphasise that the problem users care about is not the double printing, but the fact that the -work flag does not do what it's intended to do: namely, to preserve the contents of a failing script's work directory for troubleshooting.

Happy to work on a PR along your suggested lines, unless you prefer to reserve this enjoyment to yourself.

@myitcv
Copy link
Collaborator

myitcv commented Jul 30, 2022

Happy to work on a PR along your suggested lines

Very happy for you to take this, thanks for checking!

@jpluscplusm
Copy link

there is a bug however in that only the first temporary directory is retained; the second should also be retained, because that's the more interesting of the two.

I came here to file an Issue about -work not retaining a .txtar's -- files -- and the results of the commands in its preamble. I couldn't find a separate issue - do folks feel /this/ issue tracks that problem adequately, or would a new Issue be useful?

@myitcv
Copy link
Collaborator

myitcv commented Jul 3, 2023

I think this issue can be used for the aspects described above, so a PR is probably the next step.

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

No branches or pull requests

3 participants