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

[WIP] docs: add testResources JSON $schema #255

Conversation

jkowalleck
Copy link
Member

@jkowalleck jkowalleck commented Jun 21, 2023

  • add $schema to json test resources
  • fix strict $schema enum

part of #254
blocked by #256
supersedes #138

@jkowalleck jkowalleck changed the title Chore test resources json add schema docs: add testResources JSON $schema Jun 21, 2023
@jkowalleck
Copy link
Member Author

jkowalleck commented Jun 21, 2023

failing tests showed the importance to fix the $schema enum.
Therefore, I plucked #138 via 0638f01

@jkowalleck jkowalleck marked this pull request as draft June 21, 2023 15:47
@jkowalleck
Copy link
Member Author

jkowalleck commented Jun 21, 2023

the Java tests still fail.
i assume they do not use the shipped schema files, but the ones found online, or the ones shipped with Java package org.cyclonedx.

how should it be spossible to test the schema against the examples, if the java test do not run on the schema files in this repo? how to test, if the schema files do change? how to test if the schema files in this repo are any good?

The current implementation of JS and PHP uses the files in this repo for validation of the valid/invalid testResources ...

@stevespringett please help.

PS: caused #256

@stevespringett
Copy link
Member

Traditionally, how it's been working is that once changes are made to the schema development branch, those changes are mirrored to the Core Java library and published as a SNAPSHOT release. We keep the snapshots up to date with the latest schemas. It has not been synchronized.

@jkowalleck
Copy link
Member Author

jkowalleck commented Jun 22, 2023

re: #255 (comment)
this will not help this very PR or future PRs that change the schema files, does it? how would you know if the changes affected the tests, before merging them?

Therefore, #256 was created with its acceptance criteria.

@stevespringett
Copy link
Member

Why are there 144 files changed? We cannot accept this PR.

Also, why would test files be required to have the $schema? That makes absolutely no sense.

For example $schema": "http://cyclonedx.org/schema/bom-1.2b.schema.json. If this had been specified for v1.2, we would have had to change all the test files twice (a, and b). We should not have to change existing tests for a bump in schema. That defeats the purpose.

@jkowalleck
Copy link
Member Author

jkowalleck commented Jun 22, 2023

re: #255 (comment)

Also, why would test files be required to have the $schema? That makes absolutely no sense.

Please read #254 to understand the motivation. Then you will see why this makes total sense for the mentioned use case ;-)

If this had been specified for v1.2, we would have had to change all the test files twice (a, and b). We should not have to change existing tests for a bump in schema. That defeats the purpose.

Now you see the hassle everybody goes through who is already referencing the existing schemas files. If we in-place-change the $id in the schema without providing the old version still , users might have to adopt and change existing documents.
The (strict)schema pins its own namespace in downstream documents. To fix this, #138 was introduced but never got accepted.
and now we are where we are.

Why are there 144 files changed? We cannot accept this PR.

Thanks for the heads-up. This PR is a draft, it is in the making, no final solution.
There are things to solve first, before I might even continue.
Current stage showed issues with the current state of the project:

  • some tests run against different schema files (snapshots)
  • the need to modify $schema in a lot of files, because it was modified over the time.
    In contrast, the XSD does not have this behavior - 1.2 stayed the same, even we are at a 3rd iteration of 1.2`.

@jkowalleck jkowalleck changed the title docs: add testResources JSON $schema [WIP] docs: add testResources JSON $schema Dec 19, 2023
@jkowalleck
Copy link
Member Author

i might rework the content of this PR, and might come up with a followup.
goal would be, that the test files for CDX1.6+ will have a schema annotated - as of #402

jkowalleck added a commit that referenced this pull request Apr 10, 2024
fixes: #254
followup of: #403
supersedes: #255

Signed-off-by: Jan Kowalleck <[email protected]>
@jkowalleck
Copy link
Member Author

Superseded by #423

@jkowalleck jkowalleck closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants