Added manifests for tests previously described in the readmes#76
Added manifests for tests previously described in the readmes#76adanilenka merged 3 commits intomainfrom
Conversation
test/rdf/from_jelly/manifest.ttl
Outdated
| ) . | ||
|
|
||
| <triples_rdf_1_1/neg_001> a jellyt:TestNegative, jellyt:TestRdfFromJelly ; | ||
| mf:name "Single frame, max_name_table_size set to 32000. For security reasons, consumers must always validate that the requested size of a prefix, name, or datatype lookup table is not overly large. The configurable max size of the name table in Jelly-JVM is up to 4096." ; |
There was a problem hiding this comment.
I know that it's an imported case, but that name table size of 32k still seems realistic. Can you change that to, say, 10 million? Then it wouldn't make sense at all.
| mf:result <triples_rdf_1_1/pos_011/out_001.nt> . | ||
|
|
||
| <triples_rdf_1_1/pos_012> a jellyt:TestPositive, jellyt:TestRdfFromJelly ; | ||
| mf:name "Two (2) frames, logical type = LOGICAL_STREAM_TYPE_GRAPHS. Prefix table disabled." ; |
There was a problem hiding this comment.
I understand that this info is in the stream options, but it would be useful to know this just by looking at the name of the test case. I'd keep it.
There was a problem hiding this comment.
in this particular case, or in every case? bc to me, the question is then why only the prefix table? Should the datatype table also be included?
There was a problem hiding this comment.
In all cases, that's really helpful.
There was a problem hiding this comment.
returned all prefix table notes in the positive test cases
Well, yeah, let's remove it.
If there is no MUST in the spec... yeah, let the consumer do whatever they like.
I'd say, "SHOULD throw an error", because the consumer could take the sensible default of just using the first stream options (which is what Jelly-JVM does). Can you make a PR to the spec to update it? |
but wouldn't using a default or the initial options threaten the validity of the stream? Bc the IDs that come in the stream later can assume different lookup tables, for instance. Like if the size of the table has changed or suddenly the prefix table starts being used. |
Yes, but that's the thing with using features undefined in the spec. We don't define the behavior, so you should not rely on implementations being consistent in that regard. Simple. |
…sage note to all positive cases
Issue #39
Also reviewed some of the tests' contents.
Propose to remove in the next review PR:
The consumer **SHOULD** throw an error if the version tag is greater than the version tag of the implementation.So it is not a requirement per se.Requires attention:
from-jelly/triples_rdf_1_1/neg_011 -- the second stream options row is not the same as the first one. Specification currently prohibits sending different stream options:
It (stream options row) MAY appear more than once in the stream (also after other rows), but it MUST be identical to all previous occurrences.But the specification does not describe what the consumer should do if a new stream options row is received and differs from the original one.As of now, jelly-cli rdf validate throws an error in this case, but jelly-cli rdf from-jelly deserializes w/o any problem. So a decision should be made on the expected consumer behaviour in this case.