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

Validate scenarios against json schema #1475

Merged
merged 16 commits into from
Aug 28, 2023
Merged

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Aug 27, 2023

Closes #1428

Since the authoritative validation of scenario files is actually performed by virtue of swarm parsing them, this CI job primarily exists to ensure the JSON Schema descriptions are accurate. This is important for two purposes:

Testing

Verified that the schema checker action does indeed work by intentionally pushing an invalid scenario file in f789f81.

@kostmo kostmo added the Z-CI This issue is about continuous integration pipelines - GH Actions - tests and releases on server. label Aug 27, 2023
@kostmo kostmo changed the title Feature/json schema fixes Validate scenarios agaisnt json schema Aug 27, 2023
@kostmo kostmo force-pushed the feature/json-schema-fixes branch 2 times, most recently from f789f81 to b43ff86 Compare August 27, 2023 01:26
@kostmo kostmo requested review from xsebek and byorgey August 27, 2023 01:29
@kostmo kostmo marked this pull request as ready for review August 27, 2023 01:29
@kostmo kostmo changed the title Validate scenarios agaisnt json schema Validate scenarios against json schema Aug 27, 2023
Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

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

It seems I left several things unfinished in #620, thanks for picking this up @kostmo! 👍

Did you manage to integrate the schema to your editor? I use VSCodium YAML extension with this workspace config:

{
    "yaml.schemas": {
        "https://raw.githubusercontent.com/swarm-game/swarm/feature/json-schema-fixes/data/schema/scenario.json": [
            "data/scenarios/**/*.yaml"
        ],
        "https://raw.githubusercontent.com/swarm-game/swarm/feature/json-schema-fixes/data/schema/entities.json": [
            "data/entities.yaml"
        ],
        "https://raw.githubusercontent.com/swarm-game/swarm/feature/json-schema-fixes/data/schema/recipes.json": [
            "data/recipes.yaml"
        ],
    }
}

We should probably note that somewhere (CONTRIBUTING.md or editors?). I am kind of embarrassed that I only wrote that to the previous PR and can't see it in any repo docs. 😅

.github/workflows/scenario-schema.yml Outdated Show resolved Hide resolved
.github/workflows/scenario-schema.yml Show resolved Hide resolved
data/schema/recipes.json Show resolved Hide resolved
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

Where is all the stuff about subworlds? I don't see that described anywhere in the schemas, but we have testing scenarios which use them.

data/schema/planarloc.json Outdated Show resolved Hide resolved
data/schema/planarloc.json Outdated Show resolved Hide resolved
data/schema/robot.json Outdated Show resolved Hide resolved
@byorgey
Copy link
Member

byorgey commented Aug 27, 2023

I don't understand how this succeeds when the schemas have nothing about subworlds but there are scenarios which make use of subworlds. Do .yaml files still validate when they have extra stuff which is not described in the schema? That would be a problem because that is probably going to be the number one failure mode (someone adds a new feature but forgets to update the schema files).

@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Aug 28, 2023
@kostmo
Copy link
Member Author

kostmo commented Aug 28, 2023

Do .yaml files still validate when they have extra stuff which is not described in the schema?

Yes, there are apparently two ways to handle this.

  1. Add "additionalProperties": false for every object in the schema. This is what I have done.
  2. Use a schema validatation tool that allows specifying this globally. The check-jsonschema tool does not support this. Interestingly, the YAML VS Code extension does have a setting for this:
    Screenshot from 2023-08-27 18-57-09

@kostmo kostmo removed the merge me Trigger the merge process of the Pull request. label Aug 28, 2023
@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Aug 28, 2023
@mergify mergify bot merged commit bfc0c14 into main Aug 28, 2023
10 checks passed
@mergify mergify bot deleted the feature/json-schema-fixes branch August 28, 2023 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request. Z-CI This issue is about continuous integration pipelines - GH Actions - tests and releases on server.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate scenarios against schema as part of CI
3 participants