split test suite #428

jkowalleck · 2025-03-18T19:40:58Z

have a folder, that contains all test suites.
have a file for the purl core spec: PURL-SPECIFICATION.json
have a file for each purl type: PURL-TYPES.<type>.json

test-suite-data
    ├── PURL-SPECIFICATION.json
    ├── PURL-TYPES.apk.json
    ├── PURL-TYPES.bitnami.json
...
    ├── PURL-TYPES.generic.json
...
    └── PURL-TYPES.swift.json

Note

this PR does not add new or alter existing tests. it just refactors their location

fixes #427

TODO / DONE

split test suite per type
extract ALL he general cases

created via script:

#!/usr/bin/env python3

from json import loads as json_loads, dumps as json_dumps
from urllib.request import urlopen
from os.path import join
from typing import Any, List, Dict

TestCaseType = Dict[str, Any]

mixed_test_suite_url = 'https://raw.githubusercontent.com/package-url/purl-spec/refs/heads/main/test-suite-data.json'
with urlopen(mixed_test_suite_url) as mixed_test_suite_res:
    mixed_test_suite_data: List[TestCaseType] = json_loads(mixed_test_suite_res.read())

ts_types: Dict[str, List[TestCaseType]] = {'generic': []}

for tc in mixed_test_suite_data:
    if tc['type'] is None:
        print('none_case:', json_dumps(tc, indent=2), '', sep='\n')
        continue
    tc_type = str(tc['type']).lower()
    if tc_type not in ts_types:
        ts_types[tc_type] = [tc]
    else:
        ts_types[tc_type].append(tc)

for tct, tcs in ts_types.items():
    with open(join('test-suite', f'PURL-TYPES.{tct}.json'), 'wt') as tcf:
        tcf.write(json_dumps(tcs, indent=2))

test-suite/PURL-SPECIFICATION.json

matt-phylum

The core file is missing tests for canonical encoding of all characters in all the different positions. This isn't currently defined by the spec for all characters, but at least there should be tests for the characters that are defined and for some critical cases where failure to encode and decode properly results in unexpected errors or incorrect values

plus vs space (broken for at least 4 implementations)
percent signs (broken for 1 implementation)
ampersands in qualifier values (broken for 2 implementations)
a control character < 0x10 (broken for 2 implementations)
a BMP Unicode character (< U+10000, eg ☃, broken for 2 implementations)
a non-BMP Unicode character (>= U+10000, eg 💩, broken for 2 implementations).

Some of encoding cases used to be covered and are not anymore. For example, colons are now only tested for implementations that support pkg:docker and pkg:cpan. Slashes in qualifier values are only tested for implementations that support huggingface or mlflow.

matt-phylum · 2025-03-19T15:03:08Z

test-suite-data/PURL-SPECIFICATION.json

+[
+  {
+    "description": "invalid subpath - unencoded subpath cannot contain '..'",
+    "purl": "pkg:GOLANG/google.golang.org/genproto@abcdedf#/googleapis/%2E%2E/api/annotations/",


This test is doing three things. GOLANG should probably be written as golang and the subpath should not begin with /.

matt-phylum · 2025-03-19T15:09:01Z

test-suite-data/PURL-SPECIFICATION.json

+    "description": "invalid encoded colon : between scheme and type",
+    "purl": "pkg%3Amaven/org.apache.commons/io",
+    "canonical_purl": null,
+    "type": "maven",


This test doesn't work. I think you got it from elsewhere because there is already a test with this problem. type cannot be "maven" because {"type":"maven","namespace": "org.apache.commons","name":"io"} is a valid PURL and the test will fail because of "is_invalid": true. "type":"pkg:maven" or "type":null would be more accurate.

@matt-phylum Thanks for your comments on this test, which seems to be a variation of the test I added as part of my (merged) scheme PR #361. I had questions at the time re how to properly construct the input values when the PURL is meant to be invalid, and similar questions re the 3 is-invalid-true tests in my (merged) type PR #383. Subsequent discussion led to

@jkowalleck's docs: how to create invalid cases in the test-suite? #403 and

my related Clarify construction of JSON test objects #419.

Clarifying the core spec's test-construction instructions would be a valuable next step in the test-updating process. (I suspect the test-construction clarification could be done even while additional work on the "Character encoding" section continues.)

matt-phylum · 2025-03-19T15:16:20Z

test-suite-data/PURL-SPECIFICATION.json

+    "description": "invalid subpath - unencoded subpath cannot contain '..'",
+    "purl": "pkg:GOLANG/google.golang.org/genproto@abcdedf#/googleapis/%2E%2E/api/annotations/",
+    "canonical_purl": "pkg:golang/google.golang.org/genproto@abcdedf#googleapis/api/annotations",
+    "type": "golang",


What is the difference between the pkg:generic tests (currently an empty file) and the tests in this file? Should all of the tests in this file be type generic? They're cases where the package being type golang or whatever shouldn't make a difference, but some implementations validate that the package type is a supported type, and it's possible that an implementation doesn't support one of these types or doesn't support it correctly and gets an unexpected test result.

matt-phylum · 2025-03-19T15:40:10Z

test-suite-data/PURL-TYPES.maven.json

+  {
+    "description": "double slash // after scheme is not significant",
+    "purl": "pkg://maven/org.apache.commons/io",
+    "canonical_purl": "pkg:maven/org.apache.commons/io",
+    "type": "maven",
+    "namespace": "org.apache.commons",
+    "name": "io",
+    "version": null,
+    "qualifiers": null,
+    "subpath": null,
+    "is_invalid": false
+  },


Some of these tests are duplicates of core tests and aren't related to the package type.

mprpic

LGTM, I would file separate issues for the comments that @matt-phylum left in this PR since the change here is only about better organization of the test data.

jkowalleck · 2025-03-19T15:59:53Z

LGTM, I would file separate issues for the comments that @matt-phylum left in this PR since the change here is only about better organization of the test data.

@matt-phylum's comment seams to be out of scope.
this PR is not about adding any new test cases, nor is it about altering exsing ones.

jkowalleck · 2025-03-27T16:51:12Z

@pombredanne @package-url/purl-spec-helpers can we merge this one?

I mean, we could wait until all the tens of open PRs that want to modify the test suite are merged eventually, but this means we would not have a reasonably usable test suite in the meantime.
Or we might wait until we have clarification for certain edge cases in the test suite, which seams to take months.
All not desirable, I'd say.

Anyway, I am a huge fan of many small, scoped changes - like this one - so downstream users can review and adopt in small iterations, instead of one big blob of rework.

Let's merge this one, so that downstream users have an immediate benefit, even though all these other PRs are still in triage for months.
(I will then go through each one of open PRs and manually resolve the conflicts with the test-suite, if any)

PS: merging this PR now would especially enable downstream users to adopt every upcoming change to the test-suite even easier, as they can plug/choose what they need.

jkowalleck · 2025-04-02T16:39:23Z

need to incorporate #416

PS: done via f528a57

ppkarwasz · 2025-04-02T22:02:27Z

test-suite-data/PURL-TYPES.maven.json

+  },
+  {
+    "description": "maven often uses qualifiers",
+    "purl": "pkg:Maven/org.apache.xmlgraphics/[email protected]?classifier=sources&repositorY_url=repo.spring.io/release",


Isn't this an invalid example, since the solidus / in repo.spring.io/release is not percent encoded?

No. The parser is supposed to split on ? before /.

What about #439? I assume that / must be unencoded, when used as a PURL separator and encoded otherwise?

Signed-off-by: Jan Kowalleck <[email protected]>

incorporate package-url#416 Signed-off-by: Jan Kowalleck <[email protected]>

johnmhoran

Excellent idea @jkowalleck -- LGTM.

jkowalleck added the Test suite label Mar 18, 2025

jkowalleck commented Mar 18, 2025

View reviewed changes

test-suite/PURL-SPECIFICATION.json Outdated Show resolved Hide resolved

jkowalleck changed the title ~~[WIP] split test suite~~ split test suite Mar 19, 2025

jkowalleck requested review from pombredanne, johnmhoran and a team March 19, 2025 09:42

jkowalleck marked this pull request as ready for review March 19, 2025 09:43

matt-phylum reviewed Mar 19, 2025

View reviewed changes

mprpic approved these changes Mar 19, 2025

View reviewed changes

jkowalleck requested a review from a team March 19, 2025 16:03

mprpic previously approved these changes Mar 28, 2025

View reviewed changes

jkowalleck dismissed mprpic’s stale review via 598d148 April 2, 2025 14:48

ppkarwasz reviewed Apr 2, 2025

View reviewed changes

jkowalleck added 8 commits April 3, 2025 01:01

get rid of mixed test suite data

ae551fd

Signed-off-by: Jan Kowalleck <[email protected]>

auto-split

3cbdfe6

Signed-off-by: Jan Kowalleck <[email protected]>

none-types

0b80ec0

Signed-off-by: Jan Kowalleck <[email protected]>

spec cases

2ebf831

Signed-off-by: Jan Kowalleck <[email protected]>

remove duplicates

f486268

Signed-off-by: Jan Kowalleck <[email protected]>

rename

b3a6174

Signed-off-by: Jan Kowalleck <[email protected]>

docs

0d9d8e8

Signed-off-by: Jan Kowalleck <[email protected]>

Fix colon encoding in tests again

f528a57

incorporate package-url#416 Signed-off-by: Jan Kowalleck <[email protected]>

jkowalleck force-pushed the tests/split-test-suite branch from 598d148 to f528a57 Compare April 2, 2025 23:04

jkowalleck requested a review from a team April 2, 2025 23:07

johnmhoran approved these changes Apr 2, 2025

View reviewed changes

jkowalleck requested review from mprpic and a team April 2, 2025 23:14

johnmhoran mentioned this pull request Apr 3, 2025

Clarify construction of JSON test objects #419

Open

johnmhoran added this to the 1.0-draft milestone Apr 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

split test suite #428

split test suite #428

jkowalleck commented Mar 18, 2025 •

edited

Loading

matt-phylum left a comment

matt-phylum Mar 19, 2025

matt-phylum Mar 19, 2025

johnmhoran Apr 3, 2025

matt-phylum Mar 19, 2025

matt-phylum Mar 19, 2025

mprpic left a comment

jkowalleck commented Mar 19, 2025 •

edited

Loading

jkowalleck commented Mar 27, 2025 •

edited

Loading

jkowalleck commented Apr 2, 2025 •

edited

Loading

ppkarwasz Apr 2, 2025

matt-phylum Apr 2, 2025

ppkarwasz Apr 3, 2025

johnmhoran left a comment

split test suite #428

Are you sure you want to change the base?

split test suite #428

Conversation

jkowalleck commented Mar 18, 2025 • edited Loading

matt-phylum 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

mprpic left a comment

Choose a reason for hiding this comment

jkowalleck commented Mar 19, 2025 • edited Loading

jkowalleck commented Mar 27, 2025 • edited Loading

jkowalleck commented Apr 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnmhoran left a comment

Choose a reason for hiding this comment

jkowalleck commented Mar 18, 2025 •

edited

Loading

jkowalleck commented Mar 19, 2025 •

edited

Loading

jkowalleck commented Mar 27, 2025 •

edited

Loading

jkowalleck commented Apr 2, 2025 •

edited

Loading