Skip to content

Conversation

@tgiardina
Copy link
Contributor

@tgiardina tgiardina commented Sep 30, 2020

Summary

This PR replaces my our first attempt to implement base path support (#160; see also Issue #159). It accomplishes two things:

  1. Implements basePath support for OpenApi2
  2. Updates error messages so that OpenApi2 basePath and OpenApi3 server errors are handled consistently (see the discussion here).

Commits

  • The first two commits specify the desired behavior (both in terms of basePath support and error messages) via tests.
  • The third commit implements basePath support and the pertinent error messages.
  • The fourth commit moves some shared functionality into the relevant utils file.
  • The final commit adds me to AUTHORS.md.

The chai validator now uses OpenApi2's 'basePath' parameter if it is present. This commit also contains updates to the pertinent error messages.
@codecov
Copy link

codecov bot commented Sep 30, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@bdd04f2). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             master      #172   +/-   ##
==========================================
  Coverage          ?   100.00%           
==========================================
  Files             ?        33           
  Lines             ?       664           
  Branches          ?        47           
==========================================
  Hits              ?       664           
  Misses            ?         0           
  Partials          ?         0           
Impacted Files Coverage Δ
...esponse-validator/lib/assertions/satisfyApiSpec.js 100.00% <100.00%> (ø)
.../lib/openapi-validator/lib/classes/OpenApi2Spec.js 100.00% <100.00%> (ø)
.../lib/openapi-validator/lib/classes/OpenApi3Spec.js 100.00% <100.00%> (ø)
...ponse-validator/lib/openapi-validator/lib/utils.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdd04f2...7410d01. Read the comment docs.

@rwalle61
Copy link
Collaborator

rwalle61 commented Oct 1, 2020

Thanks very much! Will review when I can

Copy link
Collaborator

@rwalle61 rwalle61 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks a lot! Final few small name changes then we can merge!

Then I'll:

  • port it to the jest plugin
  • maybe rename "defined" to "provided" (see the comment below)
  • update the Contributing.md as per your suggestions
  • release :):)

@rwalle61
Copy link
Collaborator

rwalle61 commented Oct 6, 2020

Thank you for the updates! At a quick glance they look great. I'll give this a full final review asap and hopefully merge :)

Copy link
Collaborator

@rwalle61 rwalle61 left a comment

Choose a reason for hiding this comment

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

Super!

For anyone looking at this in future, this satisfies

@rwalle61 rwalle61 changed the title Impl OpenApi2 basePath support feat(chai): support openapi2 basepath Oct 7, 2020
@rwalle61 rwalle61 merged commit e91c0d5 into openapi-library:master Oct 7, 2020
rwalle61 pushed a commit that referenced this pull request Oct 11, 2020
* feat: support OpenAPI 2 basePath (#172)
* test: remove invalid test cases (#185)
* refactor: general tidy (#185)
* chore: drop unused dependency (compress-tag)
@rwalle61 rwalle61 mentioned this pull request Oct 11, 2020
rwalle61 pushed a commit that referenced this pull request Oct 11, 2020
**chai-openapi-response-validator** and **jest-openapi**

feat:
* support OpenAPI 2 basePaths #172 @tgiardina & #186 @rwalle61 
* (minor) add extra newline in error message #185 #186 @rwalle61

test:
* we dropped a few invalid test cases, which should not affect any users #185 #186 @rwalle61
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