Update Test Suite #52
Conversation
…appears to be a copy paste error
| "purl": "pkg:docker/customer/dockerimage@sha256:244fd47e07d1004f0aed9c?repository_url=gcr.io", | ||
| "canonical_purl": "pkg:docker/customer/dockerimage@sha256:244fd47e07d1004f0aed9c?repository_url=gcr.io", | ||
| "purl": "pkg:docker/customer/dockerimage@sha256%3A244fd47e07d1004f0aed9c?repository_url=gcr.io", | ||
| "canonical_purl": "pkg:docker/customer/dockerimage@sha256%3A244fd47e07d1004f0aed9c?repository_url=gcr.io", |
There was a problem hiding this comment.
This hasn't been clarified yet and is a subject of discussion in #39 which needs to be clarified before merge.
| @@ -252,7 +252,7 @@ | |||
| "is_invalid": false | |||
| }, | |||
| { | |||
| "description": "slash /// after type is not significant", | |||
| "description": "slash /// after scheme is not significant", | |||
There was a problem hiding this comment.
Thanks for the correction. This is a carryover from a time before PackageURLs began with pkg:/
| @@ -263,6 +263,30 @@ | |||
| "subpath": null, | |||
| "is_invalid": false | |||
| }, | |||
| { | |||
| "description": "slash / between version and qualifiers separator is not significant", | |||
| "purl": "pkg:maven/org.apache.commons/[email protected]/?scope=test", | |||
There was a problem hiding this comment.
The spec states that leading and trailing / is not significant on namespace and subpath only. This may be an invalid test case. Need clarification @pombredanne
There was a problem hiding this comment.
This was added as part of code coverage in the java implementation. If this is invalid - then the How to parse may contain unnecessary steps. The step after Split the remainder once from left on ':' is Strip the remainder from leading and trailing '/'. Trying to figure out what other type of URL would end up with a trailing '/' at this point in parsing.
|
this is an old PR - everything seems addressed except for the maven/java fun - can this PR be closed ? |
Likely so, but there is still some encoding issue that @johnmhoran is tracking in #344 and I created at least #361 The other topics addressed in the PR would need to be carefully reviewed and adopted or pushed back. @jeremylong IMHO there are too many important but separate concerns in the PR, and I think we should address both the test suite and the spec at once, each with small PRs. |
|
Thanks for your comment @JimFuller-RedHat. IMHO this is not ready to be closed -- it raises a version issue, a subpath issue, and an issue re that odd '/' just before the '?' separator. I am trying to address the various topics raised here -- see, e.g., the worksheet I'm drafting to track all encoding issues at once, grouped by PURL component so we can make a better informed decision. See https://docs.google.com/spreadsheets/d/1biOCUY4eCqQaYmfGDHVrASV9igYEzct6 |
|
@johnmhoran I agree with @pombredanne - would you want me to split this into multiple PRs? |
|
That would be great @jeremylong -- thank you. 👍 |
See package-url#39 Supercedes package-url#52 Signed-off-by: Jeremy Long <[email protected]>
Supercedes package-url#52 Signed-off-by: Jeremy Long <[email protected]>
Supercedes package-url#52 Signed-off-by: Jeremy Long <[email protected]>
Supercedes package-url#52 Signed-off-by: Jeremy Long <[email protected]>
supercedes package-url#52 Signed-off-by: Jeremy Long <[email protected]>
|
My belated thanks @jeremylong ! 👍 |
Correct versions that included a purl with an un-encoded colon in the version (test-suite-data.json:88).
Per purl-spec#character-encoding
Corrected copy paste issue in description of check for /// (test-suite-data.json:255).
Added test for un-necessary trailing slash (test-suite-data.json:267).
Added a bintray example to test namespaces with multiple segments (test-suite-data.json:279).
Added test case to check for invalid '..' segments in the subpath (test-suite-data.json:327).
Added test case to check for invalid '.' segments in the subpath (test-suite-data.json:339).