Some spec tests are invalid for NPM

[darkrift]

According to NPM type spec, the @ used for the scope is supposed to be encoded, however, there are 2 tests expected to pass that actually pass the @ unencoded:

purl-spec/tests/types/npm-test.json

Lines 181 to 205 in 1f8c6f2

	{
	"description": "valid npm purl without version and with subpath, scope unencoded",
	"test_group": "advanced",
	"test_type": "roundtrip",
	"input": "pkg:npm/@babel/core#/googleapis/api/annotations/",
	"expected_output": "pkg:npm/%40babel/core#googleapis/api/annotations",
	"expected_failure": false,
	"expected_failure_reason": null
	},
	{
	"description": "valid npm purl without version and with subpath, scope unencoded",
	"test_group": "advanced",
	"test_type": "parse",
	"input": "pkg:npm/@babel/core#/googleapis/api/annotations/",
	"expected_output": {
	"type": "npm",
	"namespace": "@babel",
	"name": "core",
	"version": null,
	"qualifiers": null,
	"subpath": "googleapis/api/annotations"
	},
	"expected_failure": false,
	"expected_failure_reason": null
	},

Even the npm type spec mentions they are supposed to be percent encoded :

purl-spec/types/npm-definition.json

Line 16 in 1f8c6f2

"note": "The namespace is used for the scope of a scoped NPM package. The npm scope @ sign prefix is always percent encoded, as it was in the early days of npm scope."

Is it expected and something is missing from the parsing spec or are those tests really invalid and should be either removed or modified to become valid ?

[darkrift]

@pombredanne this comes from 2821916#diff-6b10ea0c704b3c32239fc8542c710fc7482421374630d6fb74646af62daa72d7

Was this intended ? if so, why ?

[mjherzog]

@darkrift The intention here was to provide both 'advanced' tests where an implementation tool may fix/normalize errors and 'base' tests which do not fix errors. Tests in the 'base' test_group are required for conformance to the spec; tests in the 'advanced' test_group are optional.

We are working on a major upgrade of the test schema and test cases to make this distinction more clear and provide messages that document any "fix" applied for an 'advanced' test case. See Upgrade PURL test suite (view) for the scope of planned test suite upgrade/rework.

[darkrift]

@mjherzog What is the point for such implementation to "fix" those cases ? Why not just require the strict spec ?

Allowing for such implementations to "fix" errors, while helps users for simple errors, only increases complexity and removes the parsing from being completely generic.

[mjherzog]

Too long a story to cover here and I don't know all of the history. There are still many different opinions among the community about testing.
There are some major changes coming soon that will make the distinction between conformance tests (base) and other test cases (advanced) much more clear, but the principle that only the base tests are required for conformance is the best immediate guidance I can provide.

[darkrift]

Thanks for the feedback, I'll only cover the base tests for conformance then

[matt-phylum]

Isn't this wrong?

   "input": "pkg:npm/@babel/core#/googleapis/api/annotations/", 
   "expected_output": "pkg:npm/%40babel/core#googleapis/api/annotations", 

Shouldn't the expected outcome be an error because the input PURL contains no name?

[jkowalleck]

Isn't this wrong?

"input": "pkg:npm/@babel/core#/googleapis/api/annotations/",
"expected_output": "pkg:npm/%40babel/core#googleapis/api/annotations",

Shouldn't the expected outcome be an error because the input PURL contains no name?

true. from

purl-spec/docs/how-to-parse.md

Lines 52 to 57 in 18266f0

	- Split the **remainder** once from right on '@'
	- The left side is the **remainder**
	- Percent-decode the right side. This is the **version**.
	- UTF-8-decode the **version** if needed in your programming language
	- This is the **version**

when parsing the string, the purl components would be

• subpath: googleapis/api/annotations
• qualifiers: none
• scheme: pkg
• type: npm
• version: babel/core
• name: empty string
• namespace: none

[jkowalleck]

@mjherzog i think the following test case here are false-positive, they should be causing parser errors:

• purl-spec/tests/types/npm-test.json

  Lines 190 to 205 in 18266f0

  	{
  	"description": "valid npm purl without version and with subpath, scope unencoded",
  	"test_group": "advanced",
  	"test_type": "parse",
  	"input": "pkg:npm/@babel/core#/googleapis/api/annotations/",
  	"expected_output": {
  	"type": "npm",
  	"namespace": "@babel",
  	"name": "core",
  	"version": null,
  	"qualifiers": null,
  	"subpath": "googleapis/api/annotations"
  	},
  	"expected_failure": false,
  	"expected_failure_reason": null
  	},

• purl-spec/tests/types/npm-test.json

  Lines 181 to 189 in 18266f0

  	{
  	"description": "valid npm purl without version and with subpath, scope unencoded",
  	"test_group": "advanced",
  	"test_type": "roundtrip",
  	"input": "pkg:npm/@babel/core#/googleapis/api/annotations/",
  	"expected_output": "pkg:npm/%40babel/core#googleapis/api/annotations",
  	"expected_failure": false,
  	"expected_failure_reason": null
  	},

see details here: #865 (comment)