Skip to content

Conversation

@martonvago
Copy link
Contributor

Description

I refactored this wrong, sorry, fixing it here.

Needs a quick review.

Checklist

  • Formatted Markdown
  • Ran just run-all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What this function should do:

  • only remove the parent error if there are other errors in the group
  • if both data and path are missing on the resource, unite those two errors into a single better error
  • remove all data / path required errors (whether there are two errors or just one)

Comment on lines +194 to +204
def test_fail_with_only_resource_name_missing():
descriptor = example_package_descriptor()
del descriptor["resources"][0]["name"]

issues = check(descriptor)

assert len(issues) == 1
assert issues[0].jsonpath == "$.resources[0].name"
assert issues[0].type == "required"


Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was failing because the name missing makes jsonschema check against both the path-based and data-based schemas, resulting in a "data is missing" issue.

But we know that, because the path is present here, we should only be checking against the path-based schema.

@martonvago martonvago moved this from Todo to In Review in Iteration planning Oct 15, 2025
@martonvago martonvago marked this pull request as ready for review October 15, 2025 15:05
@martonvago martonvago requested a review from a team as a code owner October 15, 2025 15:05
Copy link
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

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

🎉

@lwjohnst86 lwjohnst86 merged commit b59a09d into main Oct 15, 2025
7 checks passed
@lwjohnst86 lwjohnst86 deleted the fix/path-data-required-errors branch October 15, 2025 16:32
@github-project-automation github-project-automation bot moved this from In Review to Done in Iteration planning Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants