-
-
Notifications
You must be signed in to change notification settings - Fork 216
change schemas with duplicate URIs "http://localhost:1234/folder/" #360
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
Conversation
I would be in favour of tests using unique $ids, that related somehow to the tests they came from (perhaps the test name they belong to, with whitespace removed etc). |
Yeah I think this is a reasonable thing to support if it makes things convenient for the way you load tests -- if I can though, I'd love to ask that if we do say we guarantee this now, that you add a test to the test suite sanity checks that collects all the $ids and does in fact verify they're unique. |
(That test may be tricky to write depending on your level of comfort with Python though so if it is let me know and maybe we can barter for some other piece of work and I'll hit that myself) |
708663f
to
a81c56d
Compare
updated to apply the change to older drafts. I poked a bit at collecting ids to ensure their uniqueness, but I'm not familiar enough with the jsonschema library and/or python to do this currently. |
What's the status of this? it would be great to make this change, for the sake of users who reuse the same implementation instance across all tests. |
I'll try and see if I can help add the test myself for ensuring we keep this invariant true. |
a81c56d
to
cd9a4b9
Compare
any way this can be merged as is? I agree it is better for the repo to enforce this but even if it doesn't in this PR, it is helpful that these URIs be unique. |
Done, sorry for the wait. If you don't mind please do file the follow-up issue ticket to check for this, but yeah will merge as is. |
thanks @Julian. opened that issue to follow up. |
After testing refRemote.json following this PR getting merged, I found an error with the tests -- since they did not originate with the renaming in this PR, I'll open a new issue. |
refRemote.json contains three different schemas which identify as "http://localhost:1234/folder/", leading to collisions in my schema registry when I run these tests (unless I reinitialize the registry between each test).
this changes the conflicting URIs to unique ones, and changes the
folderInteger.json
schemas referenced by subschemas of each conflictingfolder/
schema to be unique as well.this PR is in draft because I haven't applied this change to older json schema drafts, and to solicit feedback.