Uh oh!
There was an error while loading. Please reload this page.
Always use line feed for newlines in acceptance tests - #2865
Conversation
| bundle/internal/tf/schema/\*.go linguist-generated=true | ||
| go.sum linguist-generated=true | ||
| bundle/schema/jsonschema.json linguist-generated=true | ||
| acceptance/** eol=lf |
There was a problem hiding this comment.
Does it make sense to split this file? Move generated .gitattributes into cmd/.gitattributes and keep manually maintained ones at root.
Also, should this setting apply only to acceptance tests or to the whole repo?
There was a problem hiding this comment.
should this setting apply only to acceptance tests or to the whole repo?
Only acceptance tests. No reason I can think of upfront to apply to the whole repo. That could have adverse consequences of checking out the repo in windows.
There was a problem hiding this comment.
Move generated .gitattributes into cmd/.gitattributes and keep manually maintained ones at root.
Thanks, good point. It's not possible to split this file because it's generated and controlled by genkit but we can create a separate .gitattributes file in acceptance/
For code ref you can search for this in sourcegraph:
if err = render.WriteGitAttributesFile(flagDir, filenames); err != nil {
return fmt.Errorf("write git attributes: %w", err)
}
| # hello, world\n base64 encoded | ||
| [[Repls]] | ||
| Old = "aGVsbG8sIHdvcmxkCg==" | ||
| New = "[HELLO-WORLD]" |
There was a problem hiding this comment.
Why not drop this replacement as well? It's not dynamic, so a comment in the script will do.
There was a problem hiding this comment.
It's slightly more robust. A comment would be easy to miss if the actual uploaded value changes. People might miss a base64 encoding change if, for example, someone decides to refactor the test.
It's also what we do for base64 encoded strings in other places.
I don't mind either way though, happy to go with a comment too.
There was a problem hiding this comment.
For me, the less replacements there are, the easier it is to read the test output.
| # hello, world\n base64 encoded | ||
| [[Repls]] | ||
| Old = "aGVsbG8sIHdvcmxkCg==" | ||
| New = "[HELLO-WORLD]" |
There was a problem hiding this comment.
For me, the less replacements there are, the easier it is to read the test output.
| # This is important because the characters itself can be semantically important in tests. | ||
| # For example, an additional \r character in windows changes the upload payload if you are | ||
| # uploading the file's content to a workspace. | ||
| * text eol=lf |
There was a problem hiding this comment.
There are binary files in this tree (.whl files, for example).
This rule should only apply to .txt files (and others if we find they exhibit the same problem).
There was a problem hiding this comment.
Good catch, thanks! Only applying the annotation to .txt files.
## Why Based on feedback in #2720 (comment). We should always treat new lines in acceptance tests in windows as `\n`. This is important because the characters itself can be semantically important in tests. For example, an additional `\r` character in windows changes the upload payload if you are uploading the file's content to a workspace. ## Tests Existing workspace-io test should no longer need a custom override for windows.
Why
Based on feedback in #2720 (comment). We should always treat new lines in acceptance tests in windows as
\n. This is important because the characters itself can be semantically important in tests. For example, an additional\rcharacter in windows changes the upload payload if you are uploading the file's content to a workspace.Tests
Existing workspace-io test should no longer need a custom override for windows.