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

fix mulitple schema matches in oneOf based on the propertiesMatches value #648

Conversation

msivasubramaniaan
Copy link
Contributor

What does this PR do?

This PR checks the multiple schema matches under oneOf based on the sub schema properties matches value

What issues does this PR fix or reference?

redhat-developer/vscode-yaml#683
#586

Is it tested? How?

yes with test cases

@coveralls
Copy link

coveralls commented Feb 2, 2022

Coverage Status

Coverage increased (+0.003%) to 81.032% when pulling e34a2d9 on msivasubramaniaan:fix-multiple-schema-matches-in-OneOf into 613d7df on redhat-developer:main.

@evidolob
Copy link
Collaborator

evidolob commented Feb 2, 2022

@msivasubramaniaan It seems that you forgot to add tests

@msivasubramaniaan
Copy link
Contributor Author

@evidolob The test case was added https://github.com/redhat-developer/yaml-language-server/pull/623/files as part of this PR.

@evidolob
Copy link
Collaborator

evidolob commented Feb 3, 2022

Then I miss understand the case, if you add tests in #623 and with this changes tests not failing then your test is not correct, please make sure that we have test witch tests changes in this PR.

@msivasubramaniaan
Copy link
Contributor Author

Then I miss understand the case, if you add tests in #623 and with this changes tests not failing then your test is not correct, please make sure that we have test witch tests changes in this PR.

As part of #623 the given YAML matched both oneOf cases so it didn't fail with the latest fix. But redhat-developer/vscode-yaml#683 the YAML not matched both of given schema in oneOf and some properties are still missing. So with the latest fix the error count will not be 0. There are some test cases which covered the same scenario already

@evidolob evidolob merged commit 8b858ad into redhat-developer:main Feb 3, 2022
@msivasubramaniaan msivasubramaniaan deleted the fix-multiple-schema-matches-in-OneOf branch February 3, 2022 09:40
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.

3 participants