Add JSON literal tests. #559

davidlehn · 2023-03-01T07:01:25Z

Various tests of when @type of @json is used, the JSON may be an array, and the interaction with a @container of @set and @list.
Change compaction output for @json and @set with a JSON array.
Add test for many other possibilities and processing algorithms.
Propose new multiple JSON literals error code when trying to add multiple JSON literals as values for a property not declared a @set.

NOTE: This is a work-in-progress that needs feedback. HTML rebuild has not yet been done.

Please put major discussion in the issue that links here as this PR may need to be replaced.

- Various tests of when `@type` of `@json` is used, the JSON may be an array, and the interaction with a `@container` of `@set` and `@list`. - Change compaction output for `@json` and `@set` with a JSON array. - Add test for many other possibilities and processing algorithms. - Propose new `multiple JSON literals` error code when trying to add multiple JSON literals as values for a property.

gkellogg · 2024-07-29T23:38:07Z

Some of these assume changes to the algorithms that will need their own PRs:

Tests expand/er57 and er58 along with toRdf/er57 and er58 need a check to be sure that the value is an array, or generate the invalid set or list object error. The presumed semantics are that each array value becomes its own JSON value.
Test compact/ej01 is expected to raise multiple JSON literals because the term didn't use @set. This is arguably a breaking change as any data that did this before would now fail (of course two JSON values on input would now be one JSON value on output). This will certainly require checks in the compaction algorithm.
Test compact/js07 (and others) also change the behavior to preserve the array value.

gkellogg · 2024-07-31T18:39:45Z

Addresses #560.

TallTed

editorial, human-facing typo

tests/compact-manifest.html

Co-authored-by: Ted Thibodeau Jr <[email protected]>

pchampin

I disagree with tests er57 and er58 (in expand and toRdf) -- see my comments inline.

The rests of the tests make sense, but I agree with @gkellogg that some of them are breaking changes.

pchampin · 2024-08-16T10:47:38Z

tests/compact-manifest.jsonld

+      "@id": "#tjs12",
+      "@type": ["jld:PositiveEvaluationTest", "jld:CompactTest"],
+      "name": "Compact JSON literal (empty array)",
+      "purpose": "Tests compacting property with @type @json to a JSON literal (empty array).",


Suggested change

"purpose": "Tests compacting property with @type @json to a JSON literal (empty array).",

"purpose": "Tests compacting property with @type @json to a JSON literal (array with string).",

pchampin · 2024-08-16T10:47:57Z

tests/compact-manifest.jsonld

+      "@id": "#tjs13",
+      "@type": ["jld:PositiveEvaluationTest", "jld:CompactTest"],
+      "name": "Compact JSON literal (array with string)",
+      "purpose": "Tests compacting property with @type @json to a JSON literal (array with string).",


Suggested change

"purpose": "Tests compacting property with @type @json to a JSON literal (array with string).",

"purpose": "Tests compacting property with @type @json to a JSON literal (empty array).",

pchampin · 2024-08-16T10:58:28Z

tests/expand-manifest.jsonld

+    }, {
+      "@id": "#ter58",
+      "@type": ["jld:NegativeEvaluationTest", "jld:ExpandTest"],
+      "name": "Array not found for @json @set.",
+      "purpose": "The value for a @json @set term MUST be an array.",
+      "input": "expand/er57-in.jsonld",
+      "expectErrorCode": "invalid set or list object"


Suggested change

}, {

"@id": "#ter58",

"@type": ["jld:NegativeEvaluationTest", "jld:ExpandTest"],

"name": "Array not found for @json @set.",

"purpose": "The value for a @json @set term MUST be an array.",

"input": "expand/er57-in.jsonld",

"expectErrorCode": "invalid set or list object"

Duplicate id with the next one, and duplicate content with the previous one.

pchampin · 2024-08-16T11:06:49Z

tests/expand-manifest.jsonld

+      "@id": "#ter57",
+      "@type": ["jld:NegativeEvaluationTest", "jld:ExpandTest"],
+      "name": "Array not found for @json @set.",
+      "purpose": "The value for a @json @set term MUST be an array.",
+      "input": "expand/er57-in.jsonld",
+      "expectErrorCode": "invalid set or list object"


I this test is accepted, it should have counterparts with other values for @type (ex:datatype, @id), and without any @type. I see no reason why "@type": "@json" should behave differently.

But more generally, I don't think that this test should be accepted nor that this behavior should be required. There are many other situations where parts of the context are ignored if the encountered value is not of the expected kind. For example, @type coertion is ignored if the value of the property is not a JSON scalar.

pchampin · 2024-08-16T11:08:37Z

tests/expand-manifest.jsonld

+    }, {
+      "@id": "#ter58",
+      "@type": ["jld:NegativeEvaluationTest", "jld:ExpandTest"],
+      "name": "Array not found for @json @list.",
+      "purpose": "The value for a @json @list term MUST be an array.",
+      "input": "expand/er58-in.jsonld",
+      "expectErrorCode": "invalid set or list object"


Same comment as above (for ter57): I don't think that this test should be accepted.

I concur that having a simple value converted to an RDF list is a bit counter-intuitive, but it follows the principle that, in JSON-LD, "prop": val and "prop": [val] are equivalent.

pchampin · 2024-08-16T11:25:45Z

tests/fromRdf-manifest.jsonld

+      "name": "JSON literals (strings)",
+      "purpose": "Tests creating property with multiple rdf:type rdf:JSON to a JSON literal (null).",


the 'name' is not accurate, and I don't understand the sentence under 'purpose' (using "rdf:type" to talk about the datatype is very confusing).

Suggested change

"name": "JSON literals (strings)",

"purpose": "Tests creating property with multiple rdf:type rdf:JSON to a JSON literal (null).",

"name": "Multiple JSON literals",

"purpose": "Tests creating property with multiple literal values of datatype rdf:JSON",

pchampin · 2024-08-16T11:29:47Z

tests/fromRdf-manifest.jsonld

+      "name": "Mixed JSON literals and non-JSON",
+      "purpose": "Tests creating property with mixed rdf:type rdf:JSON and non-JSON.",


Suggested change

"name": "Mixed JSON literals and non-JSON",

"purpose": "Tests creating property with mixed rdf:type rdf:JSON and non-JSON.",

"name": "Mixed JSON literal and non-JSON",

"purpose": "Tests creating property with mixed values: literal of datatype rdf:type and non-JSON.",

Actually, the test is mixing a literal of datatype rdf:JSON and an IRI, so the text is still a bit misleading (I would expect "non-JSON" to mean "a literal with another datatype than rdf:JSON).

pchampin · 2024-08-16T11:31:59Z

tests/toRdf-manifest.jsonld

+    }, {
+      "@id": "#ter57",
+      "@type": ["jld:NegativeEvaluationTest", "jld:ToRDFTest" ],
+      "name": "Array not found for @json @list.",
+      "purpose": "The value for a @json @list term MUST be an array.",
+      "input": "toRdf/er57-in.jsonld",
+      "expectErrorCode": "invalid set or list object"
+    }, {
+      "@id": "#ter58",
+      "@type": ["jld:NegativeEvaluationTest", "jld:ToRDFTest" ],
+      "name": "Array not found for @json @set.",
+      "purpose": "The value for a @json @set term MUST be an array.",
+      "input": "toRdf/er58-in.jsonld",
+      "expectErrorCode": "invalid set or list object"


Suggested change

}, {

"@id": "#ter57",

"@type": ["jld:NegativeEvaluationTest", "jld:ToRDFTest" ],

"name": "Array not found for @json @list.",

"purpose": "The value for a @json @list term MUST be an array.",

"input": "toRdf/er57-in.jsonld",

"expectErrorCode": "invalid set or list object"

}, {

"@id": "#ter58",

"@type": ["jld:NegativeEvaluationTest", "jld:ToRDFTest" ],

"name": "Array not found for @json @set.",

"purpose": "The value for a @json @set term MUST be an array.",

"input": "toRdf/er58-in.jsonld",

"expectErrorCode": "invalid set or list object"

For the reasons explained above (in expand-manifest.jsonld), I believe that these tests should not be accepted.

TallTed · 2024-08-16T17:57:42Z

tests/compact-manifest.jsonld

+    }, {
+      "@id": "#tej01",
+      "@type": ["jld:NegativeEvaluationTest", "jld:CompactTest"],
+      "name": "Multiple JSON values for a JSON literal properties",


Goes with change elsewhere

Suggested change

"name": "Multiple JSON values for a JSON literal properties",

"name": "Multiple JSON values for a JSON literal property",

This was referenced Mar 1, 2023

Various @json processing issues. #560

Open

JSON Array of length 1 gets compacted into non array digitalbazaar/jsonld.js#509

Open

gkellogg added the test:missing-coverage label Jul 10, 2024

davidlehn and others added 2 commits July 29, 2024 15:51

Move some expand/toRdf tests to avoid rebase collisions.

8ec7642

gkellogg force-pushed the add-json-literal-tests branch from ad04884 to 8ec7642 Compare July 29, 2024 22:59

Automated report generation

1d35fc0

gkellogg requested review from niklasl and pchampin July 29, 2024 23:38

gkellogg added the spec:substantive label Jul 29, 2024

TallTed suggested changes Aug 1, 2024

View reviewed changes

tests/compact-manifest.html Show resolved Hide resolved

gkellogg and others added 2 commits August 1, 2024 13:15

Update tests/compact-manifest.html

c20ddc5

Co-authored-by: Ted Thibodeau Jr <[email protected]>

Automated report generation

8d1953a

BigBlueHat mentioned this pull request Aug 7, 2024

Changes to the algorithms that will need their own PRs #613

Open

pchampin requested changes Aug 16, 2024

View reviewed changes

TallTed reviewed Aug 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add JSON literal tests. #559

Add JSON literal tests. #559

davidlehn commented Mar 1, 2023

gkellogg commented Jul 29, 2024

gkellogg commented Jul 31, 2024

TallTed left a comment

pchampin left a comment

pchampin Aug 16, 2024

pchampin Aug 16, 2024

pchampin Aug 16, 2024

pchampin Aug 16, 2024

pchampin Aug 16, 2024

pchampin Aug 16, 2024

pchampin Aug 16, 2024

pchampin Aug 16, 2024

TallTed Aug 16, 2024

	"purpose": "Tests compacting property with @type @json to a JSON literal (empty array).",
	"purpose": "Tests compacting property with @type @json to a JSON literal (array with string).",

		"name": "JSON literals (strings)",
		"purpose": "Tests creating property with multiple rdf:type rdf:JSON to a JSON literal (null).",

		"name": "Mixed JSON literals and non-JSON",
		"purpose": "Tests creating property with mixed rdf:type rdf:JSON and non-JSON.",

	"name": "Multiple JSON values for a JSON literal properties",
	"name": "Multiple JSON values for a JSON literal property",

Add JSON literal tests. #559

Are you sure you want to change the base?

Add JSON literal tests. #559

Conversation

davidlehn commented Mar 1, 2023

gkellogg commented Jul 29, 2024

gkellogg commented Jul 31, 2024

TallTed left a comment

Choose a reason for hiding this comment

pchampin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment