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

Rebuild data replacement strategy to use input from OpenAPI schema #211

Merged
merged 2 commits into from
Feb 6, 2020

Conversation

brandur-stripe
Copy link
Contributor

Previously, the data replacer worked by taking input request and output
response data, and replacing values if the types matched up. This generally
worked, but would result in some inappropriate replacements for more
complex data types like arrays, as seen in [1].

In this patch we upgrade the replacement strategy so that we perform
replacements if the type of incoming request data matches what we expected
in the response based on the OpenAPI schema. This allows us, for example,
to not only check that a type is an array, but also what sort of elements
that array is supposed to contain, even if the value from our fixtures is
an empty array (which is often the case).

Fixes #210.

r? @ob-stripe
cc @stripe/api-libraries


[1] #210

Previously, the data replacer worked by taking input request and output
response data, and replacing values if the types matched up. This
generally worked, but would result in some inappropriate replacements
for more complex data types like arrays, as seen in [1].

In this patch we upgrade the replacement strategy so that we perform
replacements if the type of incoming request data matches what we
expected in the response based on the OpenAPI schema. This allows us,
for example, to not only check that a type is an array, but also what
sort of elements that array is supposed to contain, even if the value
from our fixtures is an empty array (which is often the case).

Fixes #210.

[1] #210
Copy link
Contributor

@ob-stripe ob-stripe left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for adding a bunch of tests too.

@brandur-stripe
Copy link
Contributor Author

Thanks OB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

default_tax_rates in subscription endpoint has the wrong type
4 participants