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

proposal: testing: flag to not truncate file names when logging #55976

Open
firelizzard18 opened this issue Sep 30, 2022 · 3 comments
Open

proposal: testing: flag to not truncate file names when logging #55976

firelizzard18 opened this issue Sep 30, 2022 · 3 comments
Labels
Milestone

Comments

@firelizzard18
Copy link
Contributor

firelizzard18 commented Sep 30, 2022

I propose to add a flag to go test, such as -logabspath, that will print out the absolute path of the location of a call to (*testing.T).Log and friends instead of just the file name.

*testing.T methods that log (via (*testing.T).log) print out the location of the call, truncating the filename to just the basename. This may be helpful to users but it is problematic for tools like the VSCode Go extension. The Go extension's test support uses that location information to attach a log event to the source location. The extension assumes that such calls are made from files in the same package as the test, so this works in that case. However, it fails if those methods are called from a location outside of the test's package (if that location does not also call (*testing.T).Helper(). My proposal would provide an opt-in way for tooling such as the extension to change this behavior.

@gopherbot gopherbot added this to the Proposal milestone Sep 30, 2022
@ianlancetaylor ianlancetaylor changed the title proposal: src/testing: flag to not truncate file names when logging proposal: testing: flag to not truncate file names when logging Sep 30, 2022
@apparentlymart
Copy link

apparentlymart commented Oct 4, 2022

Whenever I think about a possible new option I also like to think about whether it might be possible to change the default behavior in a reasonable way to avoid adding the new option. 😀

In this case, I wonder: would it be reasonable to change the default behavior so that files in the current package will stay as relative paths but files in any other package will always be printed as absolute paths, to avoid the ambiguity?

My sense is that a test helper that fails to call t.Helper() is probably either some old code (from before that facility existed) or a mistake, and so most of the time the author would want to update it to include t.Helper() and therefore report a more useful path. Under that assumption it could be reasonable to print a possibly-obnoxiously-long path in the unusual case where a test helper isn't marked as such, making that oversight more obvious in the testing output. (And as a bonus it avoids the need for an option!)

(I may well be missing a situation where it's desirable to intentionally produce test logging attributed to a line that isn't in the current package, in which case I'd withdraw the above suggestion.)

@firelizzard18
Copy link
Contributor Author

firelizzard18 commented Oct 4, 2022

I'd be happy with anything that allows the VSCode Go extension to unambiguously identify the correct file. I suggested a flag because that means the default is no change in behavior.

Regardless of why t.Helper() is not called, incorrect/ambiguous paths is a bad experience for the user. For log traces that come from the current package, the user can ctrl/cmd-click on the log to open the file and line. For log traces from other packages, the user tries to ctrl/cmd-click on the log file but it fails because the path is wrong. As an extension developer (I wrote the test explorer integration), I want to provide a good user experience even if the developer made a mistake (e.g. forgot to call t.Helper()).

In my own work, there is an unavoidable case where t.Helper() does not help: code running in a goroutine. Some of the tooling I use for tests runs asynchronously (and lives in a different package), so there is no current package file in the stack trace for that code. In other cases, the tooling is used for other purposes beyond just Go unit tests such that I do not want it to depend on the testing package, so errors are passed to a callback (which may call t.Log()) but it is not appropriate for the tooling to call t.Helper().

@firelizzard18
Copy link
Contributor Author

firelizzard18 commented Oct 4, 2022

In case it matters, if this proposal is accepted I am happy to submit the CL myself.

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

No branches or pull requests

3 participants