Skip to content

Add tests for latest purl-spec #202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Add tests for latest purl-spec #202

wants to merge 18 commits into from

Conversation

TG1999
Copy link
Collaborator

@TG1999 TG1999 commented Aug 6, 2025

@pombredanne
Copy link
Member

@TG1999 I am not sure this would be the way to go. I think we should instead use a git module to get the new spec tests, but we do not want to continue to extend this JSON file.

@TG1999
Copy link
Collaborator Author

TG1999 commented Aug 6, 2025

@pombredanne got it, so we need to use https://github.com/package-url/purl-spec/tree/main/tests as a git module for tests.

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
@TG1999 TG1999 force-pushed the add_tests_for_purl-spec branch from 5f658a8 to 4c237ce Compare August 7, 2025 06:44
TG1999 added 6 commits August 7, 2025 12:16
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
@TG1999 TG1999 marked this pull request as draft August 7, 2025 07:54
TG1999 added 3 commits August 7, 2025 18:36
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
@TG1999 TG1999 marked this pull request as ready for review August 7, 2025 13:42
TG1999 added 2 commits August 7, 2025 20:19
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 8, 2025
0.17.5 (2025-08-06)

- Remove support for getting download URL for qpkg in ``purl2url``.
  package-url/packageurl-python#202

0.17.4 (2025-08-05)

- Add support for getting download URL for debian, apk, qpkg in ``purl2url``.
  package-url/packageurl-python#201
TG1999 added 6 commits August 12, 2025 14:44
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
@pombredanne pombredanne requested a review from tdruez August 14, 2025 11:51
@tdruez tdruez requested review from pombredanne and removed request for tdruez August 14, 2025 15:03
Copy link
Collaborator

@keshav-space keshav-space left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @TG1999, few nits for your consideration.

@@ -0,0 +1,3 @@
[submodule "spec"]
path = spec
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're only using purl-spec test data, we should keep it inside tests/data/.

Suggested change
path = spec
path = tests/data/purl-spec/

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should not keep the complete spec in tests

Comment on lines +141 to +153
# MLflow purl names are case-sensitive for Azure ML, it is case sensitive and must be kept as-is in the package URL
# For Databricks, it is case insensitive and must be lowercased in the package URL
if isinstance(qualifiers, dict):
repo_url = qualifiers.get("repository_url")
if repo_url and "azureml" in repo_url.lower():
return name_str
if repo_url and "databricks" in repo_url.lower():
return name_str.lower()
if isinstance(qualifiers, str):
if "azureml" in qualifiers.lower():
return name_str
if "databricks" in qualifiers.lower():
return name_str.lower()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should abstract this out into a separate function.

@@ -464,8 +486,21 @@ def from_string(cls, purl: str) -> Self:
if not type_ or not sep:
raise ValueError(f"purl is missing the required type component: {purl!r}.")

if not all(c in string.ascii_letters + string.digits + "-._" for c in type_):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not all(c in string.ascii_letters + string.digits + "-._" for c in type_):
valid_chars = string.ascii_letters + string.digits + ".-_"
if not all(c in valid_chars for c in type_):

Comment on lines +494 to +495
if ":" in type_:
raise ValueError(f"purl type cannot contain a colon: {type_!r}.")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is not reachable because : is not present in valid_chars

Comment on lines +42 to +43
with open(valid_purl_types_file, "r", encoding="utf-8") as f:
valid_purl_types = json.load(f)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we using valid_purl_types any where in test?

if expected_failure:
with pytest.raises(Exception):
PackageURL.from_string(input_str)
# assert None ==PackageURL.from_string(input_str)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# assert None ==PackageURL.from_string(input_str)

Comment on lines +111 to +117
SPEC_DIR = os.path.join(os.path.dirname(__file__), "..", "spec", "tests", "types")
spec_dict = load_spec_files(SPEC_DIR)

flattened_cases = []
for filename, cases in spec_dict.items():
for case in cases:
flattened_cases.append((filename, case["description"], case))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to the top.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants