Skip to content

Commit

Permalink
Add type hints to policy_check script and its tests
Browse files Browse the repository at this point in the history 
workflow.json always passes the shared path and file type arguments,
so they can become required arguments.
  • Loading branch information
@Dhwaniartefact
Dhwaniartefact committed Aug 14, 2024
1 parent e53229f commit 78bef8b
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 104 deletions.
20 changes: 3 additions & 17 deletions src/MCPClient/lib/clientScripts/policy_check.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def _we_check_this_type_of_file(self) -> bool:
else:
return True

def _file_is_derivative(self, for_access=False) -> bool:
def _file_is_derivative(self, for_access: bool = False) -> bool:
"""Return ``True`` if the target file is a derivative; ``False``
otherwise.
"""
Expand Down Expand Up @@ -466,29 +466,15 @@ def sip_policy_checks_dir(self) -> Optional[str]:
return self._sip_policy_checks_dir


def _get_shared_path(argv):
try:
return argv[4]
except IndexError:
return None


def _get_file_type(argv):
try:
return argv[5]
except IndexError:
return "original"


def call(jobs: List[Job]) -> None:
with transaction.atomic():
for job in jobs:
with job.JobContext(logger=logger):
file_path = job.args[1]
file_uuid = job.args[2]
sip_uuid = job.args[3]
shared_path = _get_shared_path(job.args)
file_type = _get_file_type(job.args)
shared_path = job.args[4]
file_type = job.args[5]

try:
job.set_status(
Expand Down
155 changes: 68 additions & 87 deletions tests/MCPClient/test_policy_check.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json
import pathlib
import uuid
from typing import Any
from unittest import mock

import policy_check
Expand Down Expand Up @@ -78,9 +79,27 @@ def access_file_format_version(
)


STDOUT = json.dumps(
{"eventOutcomeInformation": "pass", "eventOutcomeDetailNote": "a note"}
)


@pytest.fixture
def EXECUTE_OR_RUN() -> Any:
with mock.patch(
"policy_check.executeOrRun",
return_value=(
0,
STDOUT,
"",
),
):
yield


@pytest.mark.django_db
@mock.patch("policy_check.executeOrRun")
def test_policy_check_if_rules_exist(
def test_policy_check_succeeds_if_rules_exist(
execute_or_run: mock.Mock,
sip_file: models.File,
sip: models.SIP,
Expand Down Expand Up @@ -138,8 +157,15 @@ def test_policy_check_if_rules_exist(


@pytest.mark.django_db
@mock.patch("policy_check.executeOrRun")
def test_policy_checker_check_if_no_rules_exist(
@mock.patch(
"policy_check.executeOrRun",
return_value=(
1,
STDOUT,
"",
),
)
def test_policy_checker_warns_if_rules_do_not_exist(
execute_or_run: mock.Mock,
sip_file: models.File,
sip: models.SIP,
Expand All @@ -160,13 +186,6 @@ def test_policy_checker_check_if_no_rules_exist(
JobContext=mock.MagicMock(),
spec=Job,
)
expected_stdout = json.dumps(
{
"eventOutcomeInformation": "pass",
"eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"',
}
)
execute_or_run.return_value = (0, expected_stdout, "")

policy_check.call([job])

Expand All @@ -178,8 +197,15 @@ def test_policy_checker_check_if_no_rules_exist(


@pytest.mark.django_db
@mock.patch("policy_check.executeOrRun")
def test_policy_checker_check_if_file_does_not_exist(
@mock.patch(
"policy_check.executeOrRun",
return_value=(
1,
STDOUT,
"",
),
)
def test_policy_checker_warns_if_file_does_not_exist(
execute_or_run: mock.Mock,
sip_file: models.File,
sip: models.SIP,
Expand All @@ -200,13 +226,6 @@ def test_policy_checker_check_if_file_does_not_exist(
JobContext=mock.MagicMock(),
spec=Job,
)
expected_stdout = json.dumps(
{
"eventOutcomeInformation": "pass",
"eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"',
}
)
execute_or_run.return_value = (0, expected_stdout, "")

policy_check.call([job])

Expand All @@ -218,8 +237,15 @@ def test_policy_checker_check_if_file_does_not_exist(


@pytest.mark.django_db
@mock.patch("policy_check.executeOrRun")
def test_policy_checker_execute_rule_command_returns_failed(
@mock.patch(
"policy_check.executeOrRun",
return_value=(
1,
STDOUT,
"",
),
)
def test_policy_checker_fails_if_execute_rule_command_fails(
execute_or_run: mock.Mock,
sip_file: models.File,
sip: models.SIP,
Expand All @@ -241,13 +267,6 @@ def test_policy_checker_execute_rule_command_returns_failed(
JobContext=mock.MagicMock(),
spec=Job,
)
expected_stdout = json.dumps(
{
"eventOutcomeInformation": "pass",
"eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"',
}
)
execute_or_run.return_value = (1, expected_stdout, "")

policy_check.call([job])

Expand All @@ -262,8 +281,15 @@ def test_policy_checker_execute_rule_command_returns_failed(


@pytest.mark.django_db
@mock.patch("policy_check.executeOrRun")
def test_policy_checker_execute_rule_command_returns_failed_if_event_outcome_information_other_than_pass(
@mock.patch(
"policy_check.executeOrRun",
return_value=(
1,
STDOUT,
"",
),
)
def test_policy_checker_fails_if_event_outcome_information_in_output_is_not_pass(
execute_or_run: mock.Mock,
sip_file: models.File,
sip: models.SIP,
Expand All @@ -285,13 +311,6 @@ def test_policy_checker_execute_rule_command_returns_failed_if_event_outcome_inf
JobContext=mock.MagicMock(),
spec=Job,
)
expected_stdout = json.dumps(
{
"eventOutcomeInformation": "",
"eventOutcomeDetailNote": "",
}
)
execute_or_run.return_value = (0, expected_stdout, "")

policy_check.call([job])

Expand All @@ -306,11 +325,11 @@ def test_policy_check_verifies_file_type_is_preservation(
preservation_file: models.File,
sip: models.SIP,
fprule_policy_check: fprmodels.FPRule,
preservation_file_format_version,
format,
format_version,
shared_directory_path,
):
preservation_file_format_version: models.FileFormatVersion,
format: fprmodels.Format,
format_version: fprmodels.FormatVersion,
shared_directory_path: str,
) -> None:
job = mock.Mock(
args=[
"policy_check",
Expand All @@ -326,7 +345,7 @@ def test_policy_check_verifies_file_type_is_preservation(
expected_stdout = json.dumps(
{
"eventOutcomeInformation": "pass",
"eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"',
"eventOutcomeDetailNote": "a note",
}
)
execute_or_run.return_value = (0, expected_stdout, "")
Expand All @@ -346,9 +365,8 @@ def test_policy_check_verifies_file_type_is_preservation(


@pytest.mark.django_db
@mock.patch("policy_check.executeOrRun")
def test_policy_check_fails_if_file_is_not_preservation_derivative(
execute_or_run: mock.Mock,
EXECUTE_OR_RUN: mock.Mock,
preservation_file: models.File,
sip: models.SIP,
fprule_policy_check: fprmodels.FPRule,
Expand All @@ -369,13 +387,6 @@ def test_policy_check_fails_if_file_is_not_preservation_derivative(
JobContext=mock.MagicMock(),
spec=Job,
)
expected_stdout = json.dumps(
{
"eventOutcomeInformation": "pass",
"eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"',
}
)
execute_or_run.return_value = (0, expected_stdout, "")

policy_check.call([job])

Expand Down Expand Up @@ -415,7 +426,7 @@ def test_policy_check_verifies_file_type_is_access(
expected_stdout = json.dumps(
{
"eventOutcomeInformation": "pass",
"eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"',
"eventOutcomeDetailNote": "a note",
}
)
execute_or_run.return_value = (0, expected_stdout, "")
Expand Down Expand Up @@ -479,9 +490,8 @@ def test_policy_check_fails_if_file_is_not_access_derivative(


@pytest.mark.django_db
@mock.patch("policy_check.executeOrRun")
def test_policy_checker_saves_policy_check_result_into_logs_directory(
execute_or_run: mock.Mock,
EXECUTE_OR_RUN: mock.Mock,
sip_file: models.File,
sip: models.SIP,
fprule_policy_check: fprmodels.FPRule,
Expand All @@ -504,34 +514,23 @@ def test_policy_checker_saves_policy_check_result_into_logs_directory(
JobContext=mock.MagicMock(),
spec=Job,
)
expected_stdout = json.dumps(
{
"eventOutcomeInformation": "pass",
"eventOutcomeDetailNote": "MediaConch policy check result against policy file MP3 has duration: All policy checks passed: Does the audio duration exist?; MP3 has duration",
"policy": '<?xml version="1.0"?>\n<policy type="and" name="MP3 has duration" license="CC-BY-SA-4.0+">\n <description>Rudimentary test to check for an MP3 having a duration value.</description>\n <rule name="Does the audio duration exist?" value="Duration" tracktype="General" occurrence="*" operator="exists">mp3</rule>\n</policy>',
"policyFileName": "MP3 has duration",
"stdout": "<root>foobar</root>",
}
)
execute_or_run.return_value = (0, expected_stdout, "")

policy_check.call([job])

job.set_status.assert_called_once_with(0)


@pytest.mark.django_db
@mock.patch("policy_check.executeOrRun")
def test_policy_checker_saves_policy_check_result_into_submission_documentation_directory(
execute_or_run: mock.Mock,
EXECUTE_OR_RUN: mock.Mock,
sip_file: models.File,
sip: models.SIP,
fprule_policy_check: fprmodels.FPRule,
sip_file_format_version: models.FileFormatVersion,
format: fprmodels.Format,
format_version: fprmodels.FormatVersion,
shared_directory_path: pathlib.Path,
):
) -> None:
submission_documentation_directory = (
shared_directory_path / "metadata" / "submissionDocumentation"
)
Expand All @@ -548,33 +547,15 @@ def test_policy_checker_saves_policy_check_result_into_submission_documentation_
JobContext=mock.MagicMock(),
spec=Job,
)
expected_stdout = json.dumps(
{
"eventOutcomeInformation": "pass",
"eventOutcomeDetailNote": "MediaConch policy check result against policy file MP3 has duration: All policy checks passed: Does the audio duration exist?; MP3 has duration",
"policy": '<?xml version="1.0"?>\n<policy type="and" name="MP3 has duration" license="CC-BY-SA-4.0+">\n <description>Rudimentary test to check for an MP3 having a duration value.</description>\n <rule name="Does the audio duration exist?" value="Duration" tracktype="General" occurrence="*" operator="exists">mp3</rule>\n</policy>',
"policyFileName": "MP3 has duration",
"stdout": "<root>foobar</root>",
}
)
execute_or_run.return_value = (0, expected_stdout, "")

policy_check.call([job])

job.set_status.assert_called_once_with(0)


@pytest.mark.django_db
@mock.patch(
"policy_check.executeOrRun",
return_value=(
0,
json.dumps({"eventOutcomeInformation": "pass"}),
"",
),
)
def test_policy_checker_checks_manually_normalized_access_derivative_file(
execute_or_run: mock.Mock,
EXECUTE_OR_RUN: mock.Mock,
transfer: models.Transfer,
sip_file: models.File,
sip: models.SIP,
Expand Down

0 comments on commit 78bef8b

Please sign in to comment.