Skip to content

Conversation

@wtrocki
Copy link
Member

@wtrocki wtrocki commented Mar 11, 2025

Proposed changes

IPA validation needs to run during PR process to verify if:

  • IPA rule were not made error by mistake and covers existing API
  • package bump does not break spectral ruleset

PR should be never merged with failed spectral validation as it will affect and break downstream builds.

Testing

Run echo "Ensuring no error violations available in OpenAPI"
Ensuring no error violations available in OpenAPI

ipa-validation
spectral lint ./openapi/v2.yaml --ruleset=./tools/spectral/ipa/ipa-spectral.yaml

No results with a severity of 'error' found!

spectral-validation
spectral lint ./openapi/v2.yaml --ruleset=./tools/spectral/.spectral.yaml

No results with a severity of 'error' found!

@wtrocki wtrocki requested a review from a team as a code owner March 11, 2025 13:21
- name: Run IPA Spectral Tests against current raw OpenAPI
run: |
echo "Ensuring no error violations available in OpenAPI"
npm run ipa-validation
Copy link
Member Author

Choose a reason for hiding this comment

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

That does not run our "classic" spectral rules. Ideally we might want to run both?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

Choose a reason for hiding this comment

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

How does it differentiate if there are validation failures or some other error happening?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Member Author

@wtrocki wtrocki Mar 11, 2025

Choose a reason for hiding this comment

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

@lovisaberggren Good catch. I will adjust that job to run on javascript files and package.json bumps (when spectral is bumped)

@yelizhenden-mdb Error message will be different. Validation failure vs Spectral failure (which is unlikely since we try catch logic in each rule at the moment).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool, yeah that's why it didn't run in a few cases 👍

@wtrocki wtrocki force-pushed the task-ipa-tests branch 2 times, most recently from b434bfa to e9f1f0d Compare March 11, 2025 15:34
Copy link
Collaborator

@lovisaberggren lovisaberggren left a comment

Choose a reason for hiding this comment

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

Thanks!

@wtrocki wtrocki merged commit acb379a into main Mar 11, 2025
13 checks passed
@wtrocki wtrocki deleted the task-ipa-tests branch March 11, 2025 16:12
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