Skip to content

Potentially missing information in summary #213

Open
@Niceplace

Description

@Niceplace

Note: Will try to work on a minimally reproductible example in a publicly accessible repository. Opening this as-is for now just to understand if this is really an issue or not.

Context

We are testing multiple openapi diffing tools and we want to support the following use case: comparing two different versions of the same open api spec "locally". This is going to run in the CI but we want to avoid providing files as URLs to minimize potential failure points.

Assuming a repository with a openapi spec file on main, and an opened pull-request that modifies this file, the following workflow would run, checkout the main version for the open-api spec and the pull-request one and then create the summary in markdown and publish it as a PR comment:

In the code block below, the variables $REPLACE_WITH_OLD_SPEC_FILE_PATH and $REPLACE_WITH_NEW_SPEC_FILE_PATH should be replaced with relative paths of each open-api spec files, relative to the main and pr-branch folders.

name: Detect breaking API changes (pb33f/openapi-changes)
on:
  workflow_dispatch:
  pull_request:
    types: [opened, synchronize, reopened]

jobs:
  openapi_diff:
    runs-on: ubuntu-latest
    steps:    
      - name: Extract repository name
        id: extract_repo_name
        run: |
          REPO_NAME=$(echo "${{ github.repository }}" | cut -d'/' -f2)          
          echo "repo_name_without_owner=$REPO_NAME" >> "$GITHUB_OUTPUT"          

      - name: Checkout repository (main branch)
        uses: actions/checkout@v4
        with:
          ref: ${{ github.base_ref }}
          repository: ${{ github.event.pull_request.head.repo.full_name }}
          path: "main"
          sparse-checkout: pleo-${{ steps.extract_repo_name.outputs.repo_name_without_owner }}-rest
          fetch-depth: 1

      - name: Checkout repository
        uses: actions/checkout@v4
        with:
          repository: ${{ github.event.pull_request.head.repo.full_name }}
          ref: ${{ github.head_ref }}
          path: "pr-branch"

      - name: Setup openapi-changes        
        run: curl -fsSL https://pb33f.io/openapi-changes/install.sh | sh
        
      - name: Run openapi-changes        
        run: |
          openapi-changes --no-logo summary --markdown --no-color \
          main/$REPLACE_WITH_OLD_SPEC_FILE_PATH \
          pr-branch/$REPLACE_WITH_NEW_SPEC_FILE_PATH \
          > openapi_changes_summary.md
      
      - name: Display OpenAPI changes summary
        id: openapi_changes_result
        if: ${{ always() }}
        run: |          
          echo 'openapichanges_summary<<EOF' >> $GITHUB_OUTPUT
          cat openapi_changes_summary.md >> $GITHUB_OUTPUT
          echo 'EOF' >> $GITHUB_OUTPUT          

      - name: Comment PR with breaking changes report (openapi_changes)
        if: ${{ always() }}  
        uses: thollander/actions-comment-pull-request@v3
        with:
          message: "${{ steps.openapi_changes_result.outputs.openapichanges_summary }}"
          comment-tag: openapi_changes_report

Potential issue

The summary data seems misleading in terms of pointing out breaking changes related to renaming endpoints and required properties. Please refer to the screenshot below, which shows a rendering of the markdown formatted summary

Image

Reading this, I understand that @ line 62 a path was removed (I assume that this impacted the line position of everything else). The next two lines mentioning 134:3 and 124:3 actually mention the same endpoint in the original spec: it was renamed. The different line numbers are confusing but it makes sense when visually comparing the two files side by side.

Another example: with ReminderRequest, Result and ErrorResponse, we know that some were removed, but it's impossible to know which just by looking at the summary. One has to open the file and navigate to the correct line #, with the potential to encounter the same scenario where the same property being renamed is flagged as two different operations at different locations in the file.

Would it be reasonable to include more information in the summary to better help identify what content in the input file triggered these errors ? I understand that the detailed reports (JSON, HTML) provide more clarity in here but we are looking for a CI friendly solution that does not require manually downloading a file or navigating to an external system.

Happy to provide more context/info if needed + a reproductible example with a simple spec.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions