Skip to content

golden file tests fail with mismatching line endings on Windows #436

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

Closed
b-strauss opened this issue Mar 18, 2017 · 5 comments
Closed

golden file tests fail with mismatching line endings on Windows #436

b-strauss opened this issue Mar 18, 2017 · 5 comments

Comments

@b-strauss
Copy link
Contributor

There are \ns all over the codebase. This leads to most of the failed golden tests on windows.

I will add an OS check and, if we are on windows, normalize the line endings.

@b-strauss
Copy link
Contributor Author

This is more complex than I anticipated... Currently we have the following situation on Windows:

Tsickle loads .ts files from the Windows filesystem. These files contain Windows line endings \r\n. It then writes custom code on top of these with \n as seen here. The result is an output string that has both types of line endings. This breaks the golden tests, because the tests compare the reference files, loaded from the Windows filesystem (\r\n), with the rewritten Tsickle output that has mixed line endings (input files \r\n plus rewritten code with \n) by equality.

I first tried to replace the line endings in the generated output string with the current platforms line endings from node.js (os.EOL). Unfortunately JavaScript's RegExp has no support for lookbehind, which would be needed to not break usercode \ns. There are workarounds that involve reversing the string, but these lead to new problems with unicode characters.

The second thing I tried, was to replace all hardcoded \n with os.EOL. This actually worked on most golden tests (yay). But now all other tests failed because the input of these tests does not come from files but from inline template strings, and per ES6 specification multiline template strings have \n line endings on all platforms ☹️.

Conclusion
Should Tsickle generate output with the line endings of the platform it is currently running on? I think it should. I think emitting os.EOL instead of hardcoded \n is the right thing. What about inline template strings? I see two solutions, converting them to separate files or converting them to oldschool strings with os.EOL.

Any advice?

cc @evmar @mprobst

@evmar
Copy link
Contributor

evmar commented Mar 19, 2017

The tsickle output is not really intended to be human-readable, it's just a temporary file that is then fed to another compiler. With that in mind it seems harmless to me to always use \n endings.

If you are seeing \r\n in the tsickle golden files I think there's a git setting related to whether to introduce \r\n into files that don't have it.

@b-strauss
Copy link
Contributor Author

Would it be reasonable to replace the line endings after loading the golden files on Windows?

@mprobst
Copy link
Contributor

mprobst commented Mar 19, 2017 via email

@b-strauss
Copy link
Contributor Author

@mprobst yes that's what I meant. That should work.

@b-strauss b-strauss changed the title Annotator emits unix line endings ('\n') on windows golden file tests fail with mismatching line endings on Windows Mar 19, 2017
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