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

test(openapi/validate): cleanup 🧹 #520

Merged
merged 23 commits into from
Jun 23, 2022
Merged

Conversation

kanadgupta
Copy link
Member

@kanadgupta kanadgupta commented Jun 23, 2022

🧰 Changes

As part of the work I did for #518, this PR fluffs out some missing test coverage and does some tiny refactors.

🧬 QA & Testing

Do tests pass? You can see the Node 14 test coverage before and after below (coverage improvements are in lib/prepareOas.js and cmds/opneapi.js).

Before:

image

After:

image

I tried running recreating this and the API responds with a 4xx, not a 5xx in this case.
it was bugging me how we had two tests that were uncategorized and clearly fit under the "upload" describe block (definitely my bad)
This error handling was unnecessary, as shown in the test I added. The validation section above it flags this, so there theoretically shouldn't be any errors bundling that wouldn't already be flagged in the validation section above.
@kanadgupta kanadgupta added the refactor Issues about tackling technical debt label Jun 23, 2022
Base automatically changed from kanad/rm-4367-update-rdme-to-support-new-oas-upload to main June 23, 2022 17:02
after noodling with this for a bit I couldn't figure it out 😞 so adding a TODO so someone can hopefully write a test for this.
@kanadgupta kanadgupta requested a review from erunion June 23, 2022 18:20
@kanadgupta kanadgupta marked this pull request as ready for review June 23, 2022 18:20
@@ -409,7 +415,7 @@ describe('rdme openapi', () => {
.post('/api/v1/api-specification', { registryUUID })
.delayConnection(1000)
.basicAuth({ user: key })
.reply(500, errorObject);
Copy link
Member

Choose a reason for hiding this comment

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

oops

@kanadgupta kanadgupta merged commit 66e8efe into main Jun 23, 2022
@kanadgupta kanadgupta deleted the openapi/test-housekeeping branch June 23, 2022 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Issues about tackling technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants