Merge pull request crs4#141 from kikkomep/fix/reporting-inherited-profiles-issues

fix: resolve inherited profiles reporting issues and restore check skipping

Author: kikkomep

rocrate_validator/cli/commands/validate.py

@@ -277,7 +277,7 @@ def validate(ctx,
             "profile_identifier": profile_identifier,
             "requirement_severity": requirement_severity,
             "requirement_severity_only": requirement_severity_only,
-            "enable_profile_inheritance": not disable_profile_inheritance,
+            "disable_inherited_profiles_issue_reporting": disable_profile_inheritance,
             "rocrate_uri": rocrate_uri,
             "rocrate_relative_root_path": relative_root_path,
             "abort_on_first": fail_fast,

rocrate_validator/models.py

@@ -1007,26 +1007,43 @@ def _do_validate_(self, context: ValidationContext) -> bool:
 
         logger.debug("Running %s checks for Requirement '%s'", len(self._checks), self.name)
         all_passed = True
-        for check in [_ for _ in self._checks
-                      if not context.settings.skip_checks
-                      or _.identifier not in context.settings.skip_checks]:
-
+        checks_to_perform = [
+            _ for _ in self._checks
+            if not context.settings.skip_checks
+            or _.identifier not in context.settings.skip_checks
+        ]
+        for check in checks_to_perform:
             try:
                 if check.overridden and not check.requirement.profile.identifier == context.profile_identifier:
                     logger.debug("Skipping check '%s' because overridden by '%r'",
                                  check.identifier, [_.identifier for _ in check.overridden_by])
                     continue
-                context.validator.notify(RequirementCheckValidationEvent(
-                    EventType.REQUIREMENT_CHECK_VALIDATION_START, check))
+                # Determine whether to skip event notification for inherited profiles
+                skip_event_notify = False
+                if check.requirement.profile.identifier != context.profile_identifier and \
+                        context.settings.disable_inherited_profiles_issue_reporting:
+                    logger.debug("Inherited profiles reporting disabled. "
+                                 "Skipping requirement %s as it belongs to an inherited profile %s",
+                                 check.requirement.identifier, check.requirement.profile.identifier)
+                    skip_event_notify = True
+                # Notify the start of the check execution if not skip_event_notify is set to True
+                if not skip_event_notify:
+                    context.validator.notify(RequirementCheckValidationEvent(
+                        EventType.REQUIREMENT_CHECK_VALIDATION_START, check))
+                # Execute the check
                 check_result = check.execute_check(context)
                 logger.debug("Result of check %s: %s", check.identifier, check_result)
                 context.result._add_executed_check(check, check_result)
-                context.validator.notify(RequirementCheckValidationEvent(
-                    EventType.REQUIREMENT_CHECK_VALIDATION_END, check, validation_result=check_result))
+                # Notify the end of the check execution if not skip_event_notify is set to True
+                if not skip_event_notify:
+                    context.validator.notify(RequirementCheckValidationEvent(
+                        EventType.REQUIREMENT_CHECK_VALIDATION_END, check, validation_result=check_result))
                 logger.debug("Ran check '%s'. Got result %s", check.identifier, check_result)
+                # Ensure the check result is a boolean
                 if not isinstance(check_result, bool):
                     logger.warning("Ignoring the check %s as it returned the value %r instead of a boolean", check.name)
                     raise RuntimeError(f"Ignoring invalid result from check {check.name}")
+                # Aggregate the check result
                 all_passed = all_passed and check_result
                 if not all_passed and context.fail_fast:
                     break
@@ -1040,7 +1057,8 @@ def _do_validate_(self, context: ValidationContext) -> bool:
                 logger.warning("Consider reporting this as a bug.")
                 if logger.isEnabledFor(logging.DEBUG):
                     logger.exception(e)
-
+        skipped_checks = set(self._checks) - set(checks_to_perform)
+        context.result.skipped_checks.update(skipped_checks)
         logger.debug("Checks for Requirement '%s' completed. Checks passed? %s", self.name, all_passed)
         return all_passed
 
@@ -1625,7 +1643,7 @@ def __initialise__(cls, validation_settings: ValidationSettings):
         profiles = [profile]
 
         # add inherited profiles if enabled
-        if validation_settings.enable_profile_inheritance:
+        if not validation_settings.disable_inherited_profiles_issue_reporting:
             profiles.extend(profile.inherited_profiles)
         logger.debug("Inherited profiles: %r", profile.inherited_profiles)
 
@@ -1656,9 +1674,20 @@ def __initialise__(cls, validation_settings: ValidationSettings):
                     if severity < severity_validation:
                         continue
                     # count the checks
-                    requirement_checks = [_ for _ in requirement.get_checks_by_level(LevelCollection.get(severity.name))
-                                          if not _.overridden or
-                                          _.requirement.profile.identifier == target_profile_identifier]
+                    requirement_checks = [
+                        _
+                        for _ in requirement.get_checks_by_level(
+                            LevelCollection.get(severity.name)
+                        )
+                        if (
+                            not validation_settings.skip_checks
+                            or _.identifier not in validation_settings.skip_checks
+                        )
+                        and (
+                            not _.overridden
+                            or _.requirement.profile.identifier == target_profile_identifier
+                        )
+                    ]
                     num_checks = len(requirement_checks)
                     requirement_checks_count += num_checks
                     if num_checks > 0:
@@ -2325,12 +2354,20 @@ class ValidationSettings:
     #: The profile identifier to validate against
     profile_identifier: str = DEFAULT_PROFILE_IDENTIFIER
     #: Flag to enable profile inheritance
+    # Use the `enable_profile_inheritance` flag with caution: disable inheritance only if the
+    # target validation profile is fully self-contained and does not rely on definitions
+    # from inherited profiles (e.g., entities defined upstream). For modularization
+    # purposes, some base entities and properties are defined in the base RO-Crate
+    # profile and are intentionally not redefined in specialized profiles; they are
+    # required for validations targeting those specializations and therefore cannot be skipped.
+    # Nevertheless, the validator can still suppress issue reporting for checks defined
+    # in inherited profiles by setting disable_inherited_profiles_issue_reporting to `True`.
     enable_profile_inheritance: bool = True
     # Validation settings
     #: Flag to abort on first error
     abort_on_first: Optional[bool] = False
-    #: Flag to disable inherited profiles reporting
-    disable_inherited_profiles_reporting: bool = False
+    #: Flag to disable reporting of issues related to inherited profiles
+    disable_inherited_profiles_issue_reporting: bool = False
     #: Flag to disable remote crate download
     disable_remote_crate_download: bool = True
     # Requirement settings

rocrate_validator/requirements/python/__init__.py

@@ -55,6 +55,11 @@ def __init__(self,
         self._check_function = check_function
 
     def execute_check(self, context: ValidationContext) -> bool:
+        if self.requirement.profile.identifier != context.profile_identifier and \
+                context.settings.disable_inherited_profiles_issue_reporting:
+            logger.debug("Skipping requirement %s as it belongs to an inherited profile %s",
+                         self.requirement.identifier, self.requirement.profile.identifier)
+            return True
         return self._check_function(self, context)
 
 

rocrate_validator/requirements/shacl/checks.py

@@ -200,19 +200,21 @@ def __do_execute_check__(self, shacl_context: SHACLValidationContext):
             assert shape is not None, "Unable to map the violation to a shape"
             requirementCheck = SHACLCheck.get_instance(shape)
             assert requirementCheck is not None, "The requirement check cannot be None"
-            failed_requirements_checks.add(requirementCheck)
-            violations = failed_requirements_checks_violations.get(requirementCheck.identifier, None)
-            if violations is None:
-                failed_requirements_checks_violations[requirementCheck.identifier] = violations = []
-            violations.append(violation)
+            if (not shacl_context.settings.skip_checks or
+                    requirementCheck.identifier not in shacl_context.settings.skip_checks):
+                failed_requirements_checks.add(requirementCheck)
+                violations = failed_requirements_checks_violations.get(requirementCheck.identifier, None)
+                if violations is None:
+                    failed_requirements_checks_violations[requirementCheck.identifier] = (violations := [])
+                violations.append(violation)
         # sort the failed checks by identifier and severity
         # to ensure a consistent order of the issues
         # and to make the fail fast mode deterministic
         for requirementCheck in sorted(failed_requirements_checks, key=lambda x: (x.identifier, x.severity)):
             # if the check is not in the current profile
             # and the disable_inherited_profiles_reporting is enabled, skip it
             if requirementCheck.requirement.profile != shacl_context.current_validation_profile and \
-                    shacl_context.settings.disable_inherited_profiles_reporting:
+                    shacl_context.settings.disable_inherited_profiles_issue_reporting:
                 continue
             for violation in failed_requirements_checks_violations[requirementCheck.identifier]:
                 violating_entity = make_uris_relative(violation.focusNode.toPython(), shacl_context.publicID)
@@ -273,6 +275,9 @@ def __do_execute_check__(self, shacl_context: SHACLValidationContext):
             if requirementCheck.identifier not in failed_requirement_checks_notified:
                 failed_requirement_checks_notified.append(requirementCheck.identifier)
                 shacl_context.result._add_executed_check(requirementCheck, True)
+                if requirementCheck.requirement.profile != shacl_context.target_profile and \
+                        shacl_context.settings.disable_inherited_profiles_issue_reporting:
+                    continue
                 shacl_context.validator.notify(RequirementCheckValidationEvent(
                     EventType.REQUIREMENT_CHECK_VALIDATION_END, requirementCheck, validation_result=True))
                 logger.debug("Added skipped check to the context: %s", requirementCheck.identifier)

tests/unit/test_services.py

@@ -21,7 +21,7 @@
 from rocrate_validator.models import ValidationSettings
 from rocrate_validator.rocrate import ROCrateMetadata
 from rocrate_validator.services import detect_profiles, get_profiles, validate
-from tests.ro_crates import InvalidMultiProfileROC, ValidROC
+from tests.ro_crates import InvalidMultiProfileROC, ValidROC, InvalidFileDescriptorEntity
 
 # set up logging
 logger = logging.getLogger(__name__)
@@ -106,6 +106,64 @@ def test_valid_local_workflow_testing_ro_crate():
         "Expected the 'workflow-testing-ro-crate-0.1' profile"
 
 
+def test_disable_inherited_profiles_issue_reporting():
+    # Set the rocrate_uri to the workflow testing RO-Crate
+    crate_path = ValidROC().workflow_testing_ro_crate
+    logger.debug("Validating a local RO-Crate: %s", crate_path)
+
+    # First, validate with inherited profiles issue reporting enabled
+    settings = ValidationSettings(
+        rocrate_uri=crate_path,
+        disable_inherited_profiles_issue_reporting=False
+    )
+    result = validate(settings)
+    total_issues_with_inheritance = len(result.get_issues())
+    logger.debug("Total issues with inherited profiles issue reporting enabled: %d", total_issues_with_inheritance)
+
+    # Now, validate with inherited profiles issue reporting disabled
+    settings.disable_inherited_profiles_issue_reporting = True
+    result = validate(settings)
+    total_issues_without_inheritance = len(result.get_issues())
+    logger.debug("Total issues with inherited profiles issue reporting disabled: %d", total_issues_without_inheritance)
+
+    # Check that disabling inherited profiles issue reporting reduces the number of reported issues
+    assert total_issues_without_inheritance <= total_issues_with_inheritance, \
+        "Disabling inherited profiles issue reporting should not increase the number of reported issues"
+
+    # Check that all reported issues are from the main profile
+    main_profile_identifier = "workflow-testing-ro-crate-0.1"
+    for issue in result.get_issues():
+        assert issue.check.profile.identifier == main_profile_identifier, \
+            "All reported issues should belong to the main profile when inherited profiles issue reporting is disabled"
+
+
+def test_skip_pycheck_on_workflow_ro_crate():
+    # Set the rocrate_uri to the workflow testing RO-Crate
+    crate_path = InvalidFileDescriptorEntity().invalid_conforms_to
+    logger.debug("Validating a local RO-Crate: %s", crate_path)
+    settings = ValidationSettings(rocrate_uri=crate_path)
+    result = validate(settings)
+    assert not result.passed(), \
+        "The RO-Crate is expected to be invalid because of an incorrect conformsTo field and missing resources"
+    assert len(result.failed_checks) == 2, "No failed checks expected when skipping the problematic check"
+    assert any(check.identifier == "ro-crate-1.1_5.3" for check in result.failed_checks), \
+        "Expected the check 'ro-crate-1.1_5.3' to fail"
+    assert any(check.identifier == "ro-crate-1.1_12.1" for check in result.failed_checks), \
+        "Expected the check 'ro-crate-1.1_12.1' to fail"
+
+    # Perform a new validation skipping specific checks
+    settings.skip_checks = ["ro-crate-1.1_5.3", "ro-crate-1.1_12.1"]
+    result = validate(settings)
+    assert result.passed(), \
+        "The RO-Crate should be valid when skipping the checks related to the invalid file descriptor entity"
+
+    # Ensure that the skipped checks are indeed skipped
+    skipped_check_ids = {check.identifier for check in result.skipped_checks}
+    # logger.error("Skipped checks: %s", result.skipped_checks)
+    assert "ro-crate-1.1_5.3" in skipped_check_ids, "Expected check 'ro-crate-1.1_5.3' to be skipped"
+    assert "ro-crate-1.1_12.1" in skipped_check_ids, "Expected check 'ro-crate-1.1_12.1' to be skipped"
+
+
 def test_valid_local_multi_profile_crate():
     # Set the rocrate_uri to the multi-profile RO-Crate
     crate_path = InvalidMultiProfileROC().invalid_multi_profile_crate

tests/unit/test_validation_settings.py

@@ -23,26 +23,34 @@ def test_validation_settings_parse_dict():
         "profiles_path": "/path/to/profiles",
         "requirement_severity": "RECOMMENDED",
         "enable_profile_inheritance": False,
+        "disable_inherited_profiles_issue_reporting": True,
+        "skip_checks": ["check1", "check2"]
     }
     settings = ValidationSettings.parse(settings_dict)
     assert str(settings.rocrate_uri) == "/path/to/data"
     assert settings.profiles_path == "/path/to/profiles"
     assert settings.requirement_severity == Severity.RECOMMENDED
     assert settings.enable_profile_inheritance is False
+    assert settings.disable_inherited_profiles_issue_reporting is True
+    assert settings.skip_checks == ["check1", "check2"]
 
 
 def test_validation_settings_parse_object():
     existing_settings = ValidationSettings(
         rocrate_uri="/path/to/data",
         profiles_path="/path/to/profiles",
         requirement_severity=Severity.RECOMMENDED,
-        enable_profile_inheritance=False
+        enable_profile_inheritance=False,
+        disable_inherited_profiles_issue_reporting=True,
+        skip_checks=["check1", "check2"]
     )
     settings = ValidationSettings.parse(existing_settings)
     assert str(settings.rocrate_uri) == "/path/to/data"
     assert settings.profiles_path == "/path/to/profiles"
     assert settings.requirement_severity == Severity.RECOMMENDED
     assert settings.enable_profile_inheritance is False
+    assert settings.disable_inherited_profiles_issue_reporting is True
+    assert settings.skip_checks == ["check1", "check2"]
 
 
 def test_validation_settings_parse_invalid_type():
@@ -72,6 +80,17 @@ def test_validation_settings_enable_profile_inheritance():
     assert settings.enable_profile_inheritance is False
 
 
+def test_validation_settings_disable_inherited_profiles_issue_reporting():
+    settings = ValidationSettings()
+    assert settings.disable_inherited_profiles_issue_reporting is False
+
+    settings = ValidationSettings(disable_inherited_profiles_issue_reporting=True)
+    assert settings.disable_inherited_profiles_issue_reporting is True
+
+    settings = ValidationSettings(disable_inherited_profiles_issue_reporting=False)
+    assert settings.disable_inherited_profiles_issue_reporting is False
+
+
 def test_validation_settings_data_path():
     settings = ValidationSettings(rocrate_uri="/path/to/data")
     assert str(settings.rocrate_uri) == "/path/to/data"