Skip to content

[MSHARED-952] make PrettyPrintXmlWriter platform independent - #62

Merged
elharo merged 6 commits into
masterfrom
i952
Aug 24, 2020
Merged

elharo merged 6 commits into
masterfrom
i952

Conversation

@elharo

@elharo elharo commented Aug 14, 2020

Copy link
Copy Markdown
Contributor

No description provided.

@michael-o

Copy link
Copy Markdown
Member

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

@elharo

elharo commented Aug 14, 2020

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

elharo commented Aug 14, 2020

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Member

Agreed.

@elharo

elharo commented Aug 15, 2020 •

Copy link
Copy Markdown
Contributor Author

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
@jira-importer

Copy link
Copy Markdown

Resolve #273

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.

3 participants