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

Make iiif-net parse all the Manifests in the Cookbook #51

Merged
merged 4 commits into from
Oct 28, 2024

Conversation

tomcrane
Copy link
Contributor

@tomcrane tomcrane commented Oct 12, 2024

The IIIF Cookbook:
https://theseus-viewer.netlify.app/cookbook-collection.json
https://iiif.io/api/cookbook/

  1. Biggest change here is renaming Audio to Sound (the latter is correct in the spec). I considered just using the string but it breaks the pattern of using nameof to when comparing type values.
  2. This needed quite a few tweaks to custom JSON converters and a new ISource and ISourceConverter for SpecificResource
  3. Added SvgSelector

When I ran the full test suite there were loads of breaking tests - but they all seem to be string comparisons of expected and actual JSON, when the JSON is actually equivalent.

In the PMC work I used this quite a bit:
https://github.com/lateapexearlyspeed/Lateapexearlyspeed.JsonSchema?tab=readme-ov-file#lateapexearlyspeedxunitassertionjson

(see JsonAssertion.Equivalent)

Should we use something like that?

Copy link
Member

@donaldgray donaldgray left a comment

Choose a reason for hiding this comment

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

When I ran the full test suite there were loads of breaking tests - but they all seem to be string comparisons of expected and actual JSON, when the JSON is actually equivalent.

In the PMC work I used this quite a bit:
https://github.com/lateapexearlyspeed/Lateapexearlyspeed.JsonSchema?tab=readme-ov-file#lateapexearlyspeedxunitassertionjson

(see JsonAssertion.Equivalent)

Should we use something like that?

I suspect the issue is down to line endings. I added a actual.ShouldMatchJson(expected); helper but it's only used in a few places, and it's a bit rough, so sounds like that lib would help

@donaldgray donaldgray self-requested a review October 28, 2024 11:33
@donaldgray donaldgray merged commit 2d0bef3 into main Oct 28, 2024
1 check passed
@donaldgray donaldgray deleted the cookbook-recipe-tests branch October 28, 2024 11:34
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.

2 participants