draft-next drop tests asserting empty fragments in $id are valid

[notEthan]

Per json-schema-org/json-schema-spec#1291

[notEthan]

also mentioned in marksparkza/jschon#69 but seems to have not led to any action in the test suite

[jdesrosiers]

I don't think we need to remove these. Is there any reason we can't keep them, but change "valid": true to "valid": false?

[notEthan]

True, that is an option. I question the value of it - it's really just a test of the pattern keyword applied to $id, and pattern is already well-covered with its own tests. But that's true both before and after this change. I'm neutral on it, and will restore the tests with valid: false if others prefer that.

[jdesrosiers]

That's a good point. The point of this test suite isn't to test the meta-schema.

[gregsdennis]

The point of this test suite isn't to test the meta-schema.

But it is the point of the suite to verify an implementation adheres to the spec.

I agree that whether the meta-schema enforces that $id doesn't have a fragment is not subject to test. However, I still think it's the responsibility of the implementation to verify this requirement.

[jdesrosiers]

I still think it's the responsibility of the implementation to verify this requirement.

I agree, but I don't think we have the capability to check for that using the test suite. We don't have the ability to test for InvalidSchema type of failures. This test validates a schema against the meta-schema as a kind of workaround for that problem. It doesn't actually check that the implementation is capable of verifying the requirement. It's just testing the meta-schema.

[notEthan]

I'm not sure where that brings us as far as either this PR or the rest of the id.json tests that boil down to pattern matching. Should they stay?

[jdesrosiers]

I'm not sure where that brings us as far as either this PR or the rest of the id.json tests that boil down to pattern matching. Should they stay?

I'd say that they should probably be removed. It might be interesting to have a suite that specifically tests the meta-schema, but that's not what this is. @Julian, @gregsdennis, what do you think?

[Julian]

Indifferent personally -- the argument for having them (and #354 and #244 in general) is just to help start to collect these tests instead of making no progress there, but I'm not sure we've made meaningful progress there regardless, so it's not much harm removing them either.

(Tl;dr weak +0 for leaving+inverting but very weak)

[gregsdennis]

I don't think we have the capability to check for that using the test suite. We don't have the ability to test for InvalidSchema type of failures.

This is a good point.

If an invalid $id means an invalid schema, and this suite requires valid schemas, then we don't have a way to test this aside from the meta-schema, which this suite isn't intended for.

So.... yeah, I say drop them.

[jdesrosiers]

the argument for having them (and #354 and #244 in general) is just to help start to collect these tests instead of making no progress there

This is a great point that I hadn't thought of. But, I also agree that there's not terribly much there and what is there is pretty basic and unlikely to be missed if we recreate this later when we have the capability to set invalid schemas.

So, if no one minds me making the call... @notEthan, would you mind either creating an new PR or updating this one that removes all the meta-schema-based $id tests?

[notEthan]

Closing, obviated by #718