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

OAS3 integration tests and fixes SO-103 #253

Merged
merged 51 commits into from Apr 18, 2019
Merged

Conversation

XVincentX
Copy link
Contributor

@XVincentX XVincentX commented Apr 15, 2019

  • Adds some integration tests on OAS3 documents
  • Changes the response negotiation to consider the presence of a default response (which gets overridden with the requested status code)
  • Changes the media type negotiation to not throw in case there's no content for that (return empty instead)
  • Uses the noEmit flag for typechecks in CI so checks time halves.

@XVincentX XVincentX marked this pull request as ready for review April 16, 2019 09:31
@XVincentX XVincentX changed the title OAS3 integration tests and fixes OAS3 integration tests and fixes SO-103 Apr 16, 2019
@XVincentX
Copy link
Contributor Author

@philsturgeon I've gone ahead, unified the two documents (oas2 and 3) and used the same test assertions; then I wrote the required specific assertions for oas3.

Moreover, while writing these tests I literally lost 2-3 hours in understanding a subtle bug I was not able to track — ultimately it turn out that since the graphite instances created by the loaders for Prism are singleton, when loading a different file even though creating a new Prims instance the new resources get stacked up on top of the last loaded ones — basically creating a whole oas2+oas3 resources array, confusing the whole Prism server. See a33b860 for more details. //cc @chris-miaskowski this made me lose my mind for a while

Copy link
Contributor

@chris-miaskowski chris-miaskowski left a comment

Choose a reason for hiding this comment

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

Yo, good job Vincenzo! Prism it starting to look great again :)
I have only few small comments.

Copy link
Contributor

@chris-miaskowski chris-miaskowski left a comment

Choose a reason for hiding this comment

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

All good!

@XVincentX XVincentX merged commit 930d29e into develop Apr 18, 2019
@XVincentX XVincentX deleted the feat/oas3-integration-tests branch April 18, 2019 10:03
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.

None yet

3 participants