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

[MSHARED-952] make PrettyPrintXmlWriter platform independent #62

Merged
merged 6 commits into from
Aug 24, 2020
Merged

Conversation

elharo
Copy link
Contributor

@elharo elharo commented Aug 14, 2020

No description provided.

@michael-o
Copy link
Member

Isn't the test portable already when the proper line separator is used throughout?

@elharo
Copy link
Contributor Author

elharo commented Aug 14, 2020

It passes on all platforms ATM but it's not testing the same thing on all platforms. E.g. if we had a bug in handling \r\n then the bug wouldn't be seen when running on Linux or the Mac. Or if we had a bug handing \n it wouldn't be found on Windows.

@michael-o
Copy link
Member

But the purpose of the test is to test the tags and not the line separators? You have removed unifyLineSeparators() which makes it better.

@elharo
Copy link
Contributor Author

elharo commented Aug 14, 2020

The general contract of a unit test is running a function with known, fixed input and comparing it to known fixed output. unifyLineSeparators() means the input and output to which its applied are no longer fixed. They're platform dependent. While we do occasionally need tests for specific issues that crop up on this or that platform, there's no need for it here.

@michael-o
Copy link
Member

Agreed.

@elharo
Copy link
Contributor Author

elharo commented Aug 15, 2020

Fixing this has revealed some more platform dependent code in our output code which interferes with reproducible builds.

@elharo elharo changed the title [MSHARED-952] make test platform independent [MSHARED-952] make PrettyPrinXmlWriter platform independent Aug 16, 2020
@elharo elharo changed the title [MSHARED-952] make PrettyPrinXmlWriter platform independent [MSHARED-952] make PrettyPrintXmlWriter platform independent Aug 16, 2020
@elharo elharo merged commit 6852024 into master Aug 24, 2020
@elharo elharo deleted the i952 branch August 24, 2020 12:11
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

Successfully merging this pull request may close these issues.

2 participants