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

Add testcases for allOf cases #2769

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Add testcases for allOf cases #2769

merged 2 commits into from
Oct 2, 2023

Conversation

papegaaij
Copy link
Contributor

@papegaaij papegaaij commented Jun 14, 2023

fixes #2745

This PR adds 2 tests for the cases described in #2745 .

The first test defines a schema with an allOf with a single subschema in the base type. This should be treated as equivalent to not using allOf at all. Currently, this breaks the code generation for inheritance entirely.

The second test defines a type with an allOf with the properties split into 2 parts. These should be merged in the generated type, but at the moment only the second property is added.

@papegaaij
Copy link
Contributor Author

papegaaij commented Jun 14, 2023 via email

@baywet
Copy link
Member

baywet commented Jun 15, 2023

Thanks for the pull request. Obviously I can't merge it as is as it's failing for now. But it'll be really helpful during the investigation. I'll talk with the PM group to understand when we sequence the issue you've reported.

@baywet
Copy link
Member

baywet commented Jul 4, 2023

@papegaaij would you mind rebasing this branch to address the merge conflicts please?
And while you're at it, can you validate whether #2849 addresses this issue?
Thanks for your patience, we have a number of people out for vacations/holidays and we're trying to wrap up 1.4

@papegaaij papegaaij requested a review from a team as a code owner July 4, 2023 17:42
@papegaaij
Copy link
Contributor Author

I've rebased the branch. Unfortunately, the fix in #2849 does not fix the tests.

@baywet
Copy link
Member

baywet commented Sep 28, 2023

Thank you again for your patience on the matter @papegaaij
I've started looking at this one too. When merging #2782 and #2953 into this branch locally, the second unit test you've added now passes. More investigation is required.

@baywet baywet enabled auto-merge October 2, 2023 18:24
@baywet baywet merged commit b0fdf42 into microsoft:main Oct 2, 2023
167 of 168 checks passed
@papegaaij
Copy link
Contributor Author

Thanks @baywet on your work on this. We've managed to transform our openapi spec to avoid these issues, but they might bite someone else. Good to have them resolved.

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.

Polymorphic responses
2 participants