Skip to content

Fix masked license validation test names (#5257) - #5263

Open
harishwargad wants to merge 2 commits into
aboutcode-org:developfrom
harishwargad:fix/unique-license-validation-tests
Open

Fix masked license validation test names (#5257)#5263
harishwargad wants to merge 2 commits into
aboutcode-org:developfrom
harishwargad:fix/unique-license-validation-tests

Conversation

@harishwargad

@harishwargad harishwargad commented Aug 12, 2026

Copy link
Copy Markdown

Fixes #5257

Summary

This PR addresses issue #5257 where license rule file pairs mapped to the exact same dynamically generated Python validation test method names (e.g., cnri-python-1.6.1.RULE and cnri-python-1.6_1.RULE both producing cnri_python_1_6_1_rule), causing tests to overwrite/mask each other during dynamic test generation.

Building on the initial test setup from stale PR #5258, this PR completes the rule file renames.

Changes Made

  • Renamed cnri-python-1.6_1.RULE -> cnri-python-1.6-v1.RULE and proprietary_10.RULE -> proprietary_10_alt.RULE using git mv so generated safe Python method names are unique.
  • Added a regression test in tests/licensedcode/test_rule_file_names.py to ensure all rule file names generate unique safe Python method names in the future.
  • Updated CHANGELOG.rst and AUTHORS.rst.

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁
  • Updated documentation pages (if applicable)
  • Updated CHANGELOG.rst (if applicable)

Signed-off-by: @harishwargad

Signed-off-by: harishwargad <harishwargad@gmail.com>

@AyanSinhaMahapatra AyanSinhaMahapatra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @harishwargad see comments for changes

from pathlib import Path
from commoncode.text import python_safe_name

RULES_DATA_DIR = Path(__file__).parents[2] / "src" / "licensedcode" / "data" / "rules"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@@ -0,0 +1,20 @@
from collections import defaultdict

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like more the rule renames at https://github.com/aboutcode-org/scancode-toolkit/pull/5258/changes
cnri-python-1.6_1.RULE -> cnri-python-1.6_19.RULE
proprietary_10_alt.RULE -> proprietary_155.RULE

if len(rule_names) > 1
}

assert not duplicate_names, duplicate_names

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you directly use the assert on the list comprehension?

Signed-off-by: harishwargad <harishwargad@gmail.com>
@harishwargad
harishwargad force-pushed the fix/unique-license-validation-tests branch from e71c0f1 to 549666f Compare August 29, 2026 14:38
@harishwargad

Copy link
Copy Markdown
Author

Hi @AyanSinhaMahapatra,

Thanks for reviewing! I've updated the PR with your requested changes:

  1. Reused rules_data_dir imported directly from licensedcode.models.
  2. Updated rule file renames to use numeric suffixes (cnri-python-1.6_19.RULE and proprietary_155.RULE).
  3. Simplified the test assertion using a direct list comprehension.

All local pytest checks are passing cleanly. Ready for your review!

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.

Some license test files are masking each other

2 participants