diff --git a/rocrate_validator/cli/commands/validate.py b/rocrate_validator/cli/commands/validate.py index 46a55008..65547367 100644 --- a/rocrate_validator/cli/commands/validate.py +++ b/rocrate_validator/cli/commands/validate.py @@ -276,7 +276,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, diff --git a/rocrate_validator/models.py b/rocrate_validator/models.py index 82d77b8e..6fbd446c 100644 --- a/rocrate_validator/models.py +++ b/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 diff --git a/rocrate_validator/requirements/python/__init__.py b/rocrate_validator/requirements/python/__init__.py index 65887c97..b23c4a9d 100644 --- a/rocrate_validator/requirements/python/__init__.py +++ b/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) diff --git a/rocrate_validator/requirements/shacl/checks.py b/rocrate_validator/requirements/shacl/checks.py index 5fa56337..62a625e0 100644 --- a/rocrate_validator/requirements/shacl/checks.py +++ b/rocrate_validator/requirements/shacl/checks.py @@ -200,11 +200,13 @@ 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 @@ -212,7 +214,7 @@ def __do_execute_check__(self, shacl_context: SHACLValidationContext): # 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) diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index 1711c672..1aef7370 100644 --- a/tests/unit/test_services.py +++ b/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 diff --git a/tests/unit/test_validation_settings.py b/tests/unit/test_validation_settings.py index f2b61a15..9dcd2b40 100644 --- a/tests/unit/test_validation_settings.py +++ b/tests/unit/test_validation_settings.py @@ -23,12 +23,16 @@ 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(): @@ -36,13 +40,17 @@ def test_validation_settings_parse_object(): 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"