Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rocrate_validator/cli/commands/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
67 changes: 52 additions & 15 deletions rocrate_validator/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, it would be useful if you could add a note to the changelog when something is renamed - changed names often create errors in our fork and having a clear changelog helps to figure out what we need to update without looking through individual PRs.
(Last week I was caught a bit off-guard by the move to utils.io_helpers!)

#: Flag to disable remote crate download
disable_remote_crate_download: bool = True
# Requirement settings
Expand Down
5 changes: 5 additions & 0 deletions rocrate_validator/requirements/python/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
17 changes: 11 additions & 6 deletions rocrate_validator/requirements/shacl/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
60 changes: 59 additions & 1 deletion tests/unit/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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
Expand Down
21 changes: 20 additions & 1 deletion tests/unit/test_validation_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -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"
Expand Down