Skip to content

Conversation

@jeremyfiel
Copy link
Contributor

@jeremyfiel jeremyfiel commented Nov 21, 2024

This PR adds schema test coverage for the JSON Schema,

this is based on the work in the OAS and Overlay projects by @MikeRalphson and @ralfhandl

The schema tests do not cover JSON Schema 2020-12 keywords as defined by the inputs keyword. All Arazzo specific keywords are covered by the tests.

I've also included markdownlint-cli to clean up the spec file.

@jeremyfiel jeremyfiel force-pushed the feat/schema-test-coverage branch 3 times, most recently from af5f4af to 7263e27 Compare November 22, 2024 16:30
@jeremyfiel
Copy link
Contributor Author

rebased on v1.0.0-dev to resolve conflicts.

Copy link
Collaborator

@ralfhandl ralfhandl left a comment

Choose a reason for hiding this comment

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

@jeremyfiel Why is the specification markdown source changed by this PR?

@jeremyfiel
Copy link
Contributor Author

Markdown lint was added and the rebase onto v1.0.0-dev included some recent updates by Frank

I didn't change any wording on it

@ralfhandl
Copy link
Collaborator

I would separate these changes into two separate pull requests:

  • Markdown formatting
  • Schema test coverage

@jeremyfiel jeremyfiel force-pushed the feat/schema-test-coverage branch from 7263e27 to 45d61f5 Compare November 25, 2024 15:53
@jeremyfiel
Copy link
Contributor Author

  • stripped out the markdown lint updates
  • removed the json version of the schema

@jeremyfiel jeremyfiel force-pushed the feat/schema-test-coverage branch 2 times, most recently from fc6b037 to 8b2dc94 Compare November 26, 2024 02:02
Copy link
Contributor Author

@jeremyfiel jeremyfiel left a comment

Choose a reason for hiding this comment

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

mistakenly removed the original validate-markdown.yaml with mdv checks.

@jeremyfiel jeremyfiel force-pushed the feat/schema-test-coverage branch 2 times, most recently from 7046008 to 53c8b4f Compare November 26, 2024 14:50
@jeremyfiel
Copy link
Contributor Author

jeremyfiel commented Nov 26, 2024

opps. somehow the validate-markdown.yaml was deleted again. added it back.

it will be replaced and updated by #291 pr after that one is merged.

Copy link
Collaborator

@frankkilcommins frankkilcommins left a comment

Choose a reason for hiding this comment

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

Thanks @jeremyfiel - i'm requested changed on the examples to ensure they are valid (which is being fixed for the examples directly in #260 )

Can you also take a look at the merge conflicts?

@jeremyfiel jeremyfiel force-pushed the feat/schema-test-coverage branch from 53c8b4f to 8c0204f Compare December 16, 2024 22:36
@jeremyfiel
Copy link
Contributor Author

tbh, i'm not sure why this is a conflict. the file currently exists on v1.0.0-dev and is requested to be deleted in this PR. so it shouldn't conflict with anything.

* remove json version of schema
* add package-lock.json
* update test case examples to use proper ABNF syntax

reused from OAS/Overlay projects
@jeremyfiel jeremyfiel force-pushed the feat/schema-test-coverage branch from 8c0204f to 61685fe Compare December 16, 2024 22:56
@jeremyfiel
Copy link
Contributor Author

interesting. adding the file back removes the conflict. I can create a separate pr to remove the file after this is merged.. lmk if that works

@jeremyfiel
Copy link
Contributor Author

taking inspiration from @ralfhandl to update the schema test coverage threshold to 100% like this will not work for this repo because we are not testing the JSON Schema 2020-12 keywords used in inputs. I'm not sure if we want to peg it to ~63% coverage to indicate the threshold is met or not. To be discussed...

jeremyfiel added a commit to jeremyfiel/Arazzo-Specification that referenced this pull request Dec 16, 2024
@frankkilcommins frankkilcommins merged commit e5a1117 into OAI:v1.0.0-dev Dec 17, 2024
2 checks passed
jeremyfiel added a commit to jeremyfiel/Arazzo-Specification that referenced this pull request Dec 17, 2024
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