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.

Sign up for GitHub

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

Jump to bottom

frontend: Add tests for @apidevtools/swagger-parser #2822

Merged
merged 1 commit into from
Feb 7, 2025
Merged

frontend: Add tests for @apidevtools/swagger-parser #2822

merged 1 commit into from
Feb 7, 2025
+132 −0

Conversation

adwait-godbole
Copy link
Contributor

@adwait-godbole adwait-godbole commented Jan 31, 2025 •
edited
Loading

Fixes #2440

Screenshot from 2025-02-03 17-38-13

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jan 31, 2025
@adwait-godbole adwait-godbole force-pushed the feat/add-swagger-parser-test branch 2 times, most recently from a7cb7ea to ba2f4af Compare January 31, 2025 21:20
sniok
sniok requested changes Feb 3, 2025
View reviewed changes
Copy link
Contributor

@sniok sniok left a comment

Choose a reason for hiding this comment

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

Thanks for opening a PR! I think these tests are not testing the thing that was mentioned in the original issue. Could you update the tests so they check "@apidevtools/swagger-parser" output?

frontend/src/lib/docs.test.ts Outdated Show resolved Hide resolved
frontend/src/lib/docs.ts Outdated Show resolved Hide resolved
@adwait-godbole adwait-godbole force-pushed the feat/add-swagger-parser-test branch from ccf69e7 to 3a0ed7e Compare February 3, 2025 12:13
@adwait-godbole
Copy link
Contributor Author

adwait-godbole commented Feb 3, 2025 •
edited
Loading

Hi @sniok, Thanks for the review! I apologize for misunderstanding the issue.

I have now updated the tests to actually test the output of @apidevtools/swagger-parser library.

We are primarily using this library for dereferencing Kubernetes OpenAPI Spec Documents and now I am testing this behavior under various dereferencing conditions. I have updated the screenshot in the PR description. Let me know what you think. Thanks.

@adwait-godbole adwait-godbole force-pushed the feat/add-swagger-parser-test branch from e93392a to 79164ce Compare February 3, 2025 12:21
@adwait-godbole adwait-godbole requested a review from sniok February 3, 2025 12:21
@adwait-godbole adwait-godbole force-pushed the feat/add-swagger-parser-test branch from 65047ff to e8ac58f Compare February 3, 2025 12:50
@adwait-godbole adwait-godbole changed the title frontend: Add test for @apidevtools/swagger-parser frontend: Add tests for @apidevtools/swagger-parser Feb 3, 2025
sniok
sniok approved these changes Feb 6, 2025
View reviewed changes
Copy link
Contributor

@sniok sniok 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!

illume
illume approved these changes Feb 7, 2025
View reviewed changes
Copy link
Collaborator

@illume illume left a comment

Choose a reason for hiding this comment

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

🎉🎈

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 7, 2025
@illume illume merged commit f1fa516 into headlamp-k8s:main Feb 7, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@illume illume illume approved these changes

@sniok sniok sniok approved these changes

Assignees
No one assigned
Labels
lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add test for @apidevtools/swagger-parser
3 participants
@adwait-godbole @illume @sniok