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

Fixed tests on windows #207

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixed tests on windows #207

wants to merge 2 commits into from

Conversation

arjanmels
Copy link

Tests failed on windows due to incorrect path to data files (starting with /) and missing handling of \r\n

Copy link
Owner

@srawlins srawlins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, sorry for the late review!

@@ -56,7 +56,7 @@ class LocationDescriptionDatabase {
LocationDescriptionDatabase(this.locations);

factory LocationDescriptionDatabase.fromString(String data) {
final lines = data.split('\n');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary given the fix in tool/encode_tzf.dart?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this change modified the line ends from windows convention (CR+LF='\r\n') to Unix (LF only='\n'), before the split on LF only. Without this line there are CR remaining in the strings.

The change in encode_tzf.dart changes backward slashes '' with forward slashes '/' in the names of the timezones. (The backward slashes are coming form the filepaths on windows.)

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