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

Compare Parameters also on path level #470

Closed
tomaszhrem opened this issue Jan 9, 2024 · 5 comments · Fixed by #479
Closed

Compare Parameters also on path level #470

tomaszhrem opened this issue Jan 9, 2024 · 5 comments · Fixed by #479
Assignees
Labels
bug Something isn't working

Comments

@tomaszhrem
Copy link

tomaszhrem commented Jan 9, 2024

Is your feature request related to a problem? Please describe.
When comparing two paths parameters are checked only when they are placed directly in operation. When we have common set placed in path then text diff don't detect any change.

Describe the solution you'd like
When comparing two operations list of parameters is merged from "parent" path and given operation so we can detect changes.

Describe alternatives you've considered
Other solution can be to leave current operation to operation parameters comparison but also compare parameters between paths when using text output.

@reuvenharrison
Copy link
Collaborator

Hi @tomaszhrem and thanks for reporting the issue.
It would be very helpful if you could provide an example demonstrating this behavior.

@tomaszhrem
Copy link
Author

tomaszhrem commented Jan 10, 2024

Hey, while preparing example files I noticed that most probably I ​incorrectly understood output from diff. E.g. No endpoint changes doesn't mean there are no changes as there is also No changes result.

For now its seems that diff detects difference in paths but I was checking only if endpoints are the same. Need to dig more on how also check paths result.

For sure if base file have parameters and in revision we move it to operation then in fact API stays the same but diff detects it as endpoint modification (New path param).

I will dig more and get back to you with detailed sum up.

@tomaszhrem
Copy link
Author

tomaszhrem commented Jan 10, 2024

In mean time - should text output print No endpoint changes if some paths are changed and in fact modifies operation/endpoint?
Example below when path params are removed in revision:
path_base.txt
path_revision.txt

% oasdiff diff path_base.txt path_revision.txt -f yaml
info:
    contact:
        url:
            from: https://test.com
            to: https://test.com22
paths:
    modified:
        /admin/v0/abc/{id}:
            parameters:
                deleted:
                    header:
                        - tenant-id
                    path:
                        - id

VS

% oasdiff diff path_base.txt path_revision.txt -f text
No endpoint changes

@reuvenharrison
Copy link
Collaborator

Indeed, this is a bug.
To fix it we need to combine parameters from the path level and the operation level before diff.
Here's another symptom of the same limitation: #378

@reuvenharrison reuvenharrison added the bug Something isn't working label Jan 11, 2024
@tomaszhrem
Copy link
Author

Thx.
I update description to point that its text output which misses path parameters diff.
Also like you mentioned combination of path and operations params would solve false positive problem when in base we have param in path and in revision its moved to operation (or other way).

@reuvenharrison reuvenharrison self-assigned this Jan 30, 2024
@reuvenharrison reuvenharrison linked a pull request Jan 30, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants