Navigation Menu

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

testing: Testable examples collapse Output newlines, making them impossible to pass #59191

Open
keilin-anz opened this issue Mar 23, 2023 · 5 comments · May be fixed by #59211
Open

testing: Testable examples collapse Output newlines, making them impossible to pass #59191

keilin-anz opened this issue Mar 23, 2023 · 5 comments · May be fixed by #59211
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@keilin-anz
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.20.2 darwin/amd64

⚠️ This also appears to affect go1.19, though I believe it worked at some point in the past

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

❌ Not relevant, this happens in multiple environments including Go Playground with Go 1.19 and Go 1.20

What did you do?

  1. Provide a testable example of a function which returns output containing successive newlines
  2. Example cannot pass tests, since the Output annotation is modified by the testing parser

Minimal example

Viewable here: https://go.dev/play/p/LFktKVr75lz

✅ The ExampleFoo function returns data containing successive newlines:

Single newline:

Double newline:


Final text

✅ The ExampleFoo contains an embedded Output: annotation with the same expected output:

	//Output:
	//Single newline:
	//
	//Double newline:
	//
	//
	//Final text

✅ The Output and stdout are compared, with leading and trailing whitespace removed
❌ Multiple newlines are collapsed into single newlines, making the test fail - only solution is to modify the function under test (which is not an option)

What did you expect to see?

Output: should be parsed as-is, not modified (beyond trimming leading and trailing whitespace)

What did you see instead?

The multiple empty lines are collapsed into a single empty line

Additional context

Did a little hunting to see if I could find a root cause, no success yet

Currently suspect it's either hidden somewhere within the testing package, or is an unintended side effect of how chains of single line comments are parsed by ast

@keilin-anz
Copy link
Author

Potential solutions

  1. Make the testing library collapse newlines in both the stdout result (AKA got:) as well as the Output: comments (AKA want:)
  2. Find root cause of the newline collapse and revert it
    1. Uncertain when this started happening, so unsure of the potential impact of the change

I'm guessing (1) has the least likelihood of breaking things for people, but it is still non-optimal - since it's not technically testing that output is correct (in some scenarios it's vitally important to be sure the output contains multiple newlines, so the test is effectively not a test.. whilst that clearly should be covered by unit tests - it's still odd to have a formalised test scenario which ignores the validity of output)

@seankhliao
Copy link
Member

2 should be the way forward.
Maybe it's using the new structured doc parser that collapses newlines?

@heschi
Copy link
Contributor

heschi commented Mar 23, 2023

cc @bcmills

@heschi heschi added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 23, 2023
@heschi heschi added this to the Go1.21 milestone Mar 23, 2023
@bcmills
Copy link
Contributor

bcmills commented Mar 23, 2023

The documented behavior is that “The comparison ignores leading and trailing space”, so collapsing interior newlines does seem incorrect.

However, the Output: parsing does at least need to account for different line endings on different platforms (\n vs. \r\n).

To minimize the number of existing examples that break, I think this should probably be gated on a GODEBUG setting (see #56986).

@bcmills bcmills added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Mar 23, 2023
@bcmills bcmills modified the milestones: Go1.21, Backlog Mar 23, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 23, 2023
@gopherbot
Copy link

Change https://go.dev/cl/479119 mentions this issue: go/ast: allow Example output to not reduce empty lines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants