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

Always retain ref if exist #99

Merged
merged 1 commit into from
Nov 17, 2016
Merged

Always retain ref if exist #99

merged 1 commit into from
Nov 17, 2016

Conversation

freenerd
Copy link
Contributor

An oversight in #91

It would actually be nice to UnitTest the Roadclass to document all cases we expect.

freenerd added a commit that referenced this pull request Nov 16, 2016
@freenerd
Copy link
Contributor Author

Added a test suite because I like tests.

Copy link
Contributor

@1ec5 1ec5 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 thorough tests! Code coverage is looking a lot better in RouteStep.swift now.

r = Road(name: "Way Name", ref: nil, destination: nil, rotaryName: nil)
XCTAssertEqual(r.names!, [ "Way Name" ])
XCTAssertNil(r.codes)
r = Road(name: "Way Name 1; Way Name 2", ref: nil, destination: nil, rotaryName: nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a test for values delimited by a colon but not a space, since that's the usual OSM tagging convention.


// Name only
r = Road(name: "Way Name", ref: nil, destination: nil, rotaryName: nil)
XCTAssertEqual(r.names!, [ "Way Name" ])
Copy link
Contributor

Choose a reason for hiding this comment

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

Force-unwrapping names here will cause the test case to crash on this line if names is nil for some reason. Instead, fall back to an empty array so that this assertion fails but the test can continue and report about other failures.

@freenerd
Copy link
Contributor Author

@1ec5 thanks, feedback incorporated

Add a test suite for Road
@freenerd freenerd merged commit 679728c into master Nov 17, 2016
@freenerd
Copy link
Contributor Author

This also made it into the swift3 branch now ceaf58b

@freenerd freenerd deleted the retain-ref-always branch November 17, 2016 10:54
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