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

Fix modeled_types tests #483

Merged
merged 1 commit into from
Nov 6, 2019
Merged

Fix modeled_types tests #483

merged 1 commit into from
Nov 6, 2019

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Nov 6, 2019

Tests were failing because the model changed to using SingleLineString but
tests weren't updated and were still using String.

This updates modeled_types to use TryFrom as the primary source of conversion
logic, delegating to it in Deserialize. This simplifies using the new types in
tests, and tests our logic rather than serde. Controller tests can then use
try_into to get the expected type.

Because we're no longer diverting through serde, we no longer need to awkwardly
quote data strings in the tests.


Sorry, I didn't catch this because tests stopped running during build.

Tests now pass.

@tjkirch tjkirch requested a review from zmrow November 6, 2019 18:55
Tests were failing because the model changed to using SingleLineString but
tests weren't updated and were still using String.

This updates modeled_types to use TryFrom as the primary source of conversion
logic, delegating to it in Deserialize.  This simplifies using the new types in
tests, and tests our logic rather than serde.  Controller tests can then use
try_into to get the expected type.

Because we're no longer diverting through serde, we no longer need to awkwardly
quote data strings in the tests.
@tjkirch
Copy link
Contributor Author

tjkirch commented Nov 6, 2019

This push fixes tests in thar-be-settings that also used SingleLineString. I confirmed that all first-party tests are now passing.

Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

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

🦃

@tjkirch tjkirch merged commit fe1479d into develop Nov 6, 2019
@tjkirch tjkirch deleted the apitests branch November 6, 2019 23:09
@iliana iliana added this to the v0.2.0 milestone Nov 19, 2019
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