Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ✨ collect all Sprout-specific property errors #964

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
55c03c6
refactor: :recycle: use custom CheckError in checks
martonvago Dec 18, 2024
958bb18
feat: :sparkles: improve CheckError
martonvago Dec 19, 2024
a619ded
test: :white_check_mark: add tests for helper functions
martonvago Dec 19, 2024
3586c51
refactor: :recycle: return early from function
martonvago Dec 19, 2024
d3de146
docs: :memo: update test docstrings
martonvago Dec 19, 2024
05e7d41
apply suggestions from code review
martonvago Jan 10, 2025
52d3a24
chore(pre-commit): :pencil2: automatic fixes
pre-commit-ci[bot] Jan 10, 2025
64d7a87
Merge branch 'main' into feat/check-error
martonvago Jan 10, 2025
8196dc0
feat: :sparkles: put required fields into constants
martonvago Jan 10, 2025
8c701d8
feat: :sparkles: add simple helper functions
martonvago Jan 10, 2025
b6a5a86
feat: :sparkles: add functions to check required and blank fields
martonvago Jan 10, 2025
e9da123
feat: :sparkles: add functions for collecting Sprout-specific package…
martonvago Jan 10, 2025
a08ce8b
apply suggestions from code review
martonvago Jan 13, 2025
1dd1e1e
refactor: :recycle: rename function to validation_errors_to_check_errors
martonvago Jan 13, 2025
c595aa9
refactor: :recycle: rename file to validation_errors_to_check_errors
martonvago Jan 13, 2025
cb49bd1
docs: :memo: add more detail to docstring
martonvago Jan 13, 2025
d603d82
refactor: :recycle: rename constant to PACKAGE_RECOMMENDED_FIELDS
martonvago Jan 14, 2025
abfc4c5
fix: :bug: include `data` in resource required fields
martonvago Jan 14, 2025
6611019
refactor: :recycle: make structure of PACKAGE_SPROUT_REQUIRED_FIELDS …
martonvago Jan 14, 2025
fea4a5f
refactor: :recycle: drop fields using pop
martonvago Jan 14, 2025
1cbec8a
apply suggestions from code review
martonvago Jan 14, 2025
c5b784f
refactor: :recycle: rename function to get_sprout_specific_resource_e…
martonvago Jan 14, 2025
c854a2d
refactor: :recycle: rename file to get_sprout_specific_resource_errors
martonvago Jan 14, 2025
ee5edf0
docs: :memo: update test names and docstrings
martonvago Jan 14, 2025
ee05805
Merge branch 'feat/check-error' into feat/sprout-checks-1-required-fi…
martonvago Jan 14, 2025
e43d13e
Merge branch 'feat/sprout-checks-1-required-fields' into feat/sprout-…
martonvago Jan 14, 2025
b174133
Merge branch 'feat/sprout-checks-2-simple-helper-functions' into feat…
martonvago Jan 14, 2025
562f93b
Merge branch 'feat/sprout-checks-3-required-and-blank-checks' into fe…
martonvago Jan 14, 2025
569b947
refactor: :recycle: rename function to exclude_non_sprout_resource_er…
martonvago Jan 14, 2025
87418fb
refactor: :recycle: rename file to exclude_non_sprout_resource_errors
martonvago Jan 14, 2025
72db9d8
Merge branch 'feat/sprout-checks-2-simple-helper-functions' into feat…
martonvago Jan 14, 2025
99c783d
Merge branch 'feat/sprout-checks-3-required-and-blank-checks' into fe…
martonvago Jan 14, 2025
144b12f
apply suggestions from code review
martonvago Jan 15, 2025
0e3a772
refactor: :recycle: update test names and docstrings
martonvago Jan 15, 2025
cad7d43
docs: :memo: update test names and docstrings
martonvago Jan 17, 2025
f017c19
Merge branch 'feat/sprout-checks-3-required-and-blank-checks' into fe…
martonvago Jan 17, 2025
b33c387
Merge branch 'main' into feat/sprout-checks-2-simple-helper-functions
lwjohnst86 Jan 20, 2025
f0510d8
Merge branch 'main' into feat/sprout-checks-2-simple-helper-functions
lwjohnst86 Jan 20, 2025
7c19388
refactor: :recycle: rename functions
martonvago Jan 20, 2025
abc47e9
refactor: :recycle: rename files
martonvago Jan 20, 2025
a0d4897
refactor: :recycle: update error message and assertions
martonvago Jan 20, 2025
fd6282c
refactor: :recycle: pull out type error message into constant
martonvago Jan 20, 2025
7881d41
Merge branch 'feat/sprout-checks-2-simple-helper-functions' into feat…
martonvago Jan 20, 2025
f531760
refactor: :recycle: pull out error messages into constants
martonvago Jan 20, 2025
e5cefa8
refactor: :recycle: rename tests
martonvago Jan 20, 2025
cc9f4ba
Merge branch 'feat/sprout-checks-3-required-and-blank-checks' into fe…
martonvago Jan 20, 2025
8e6a91e
refactor: :recycle: fix imports after rename
martonvago Jan 20, 2025
2514897
Merge branch 'main' into feat/sprout-checks-4-sprout-specific-package…
lwjohnst86 Jan 21, 2025
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
29 changes: 29 additions & 0 deletions seedcase_sprout/core/sprout_checks/get_sprout_package_errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
from seedcase_sprout.core.checks.check_error import CheckError
from seedcase_sprout.core.sprout_checks.check_fields_present import (
check_fields_present,
)
from seedcase_sprout.core.sprout_checks.check_required_package_properties_not_blank import ( # noqa: E501
check_required_package_properties_not_blank,
)
from seedcase_sprout.core.sprout_checks.required_fields import (
PACKAGE_SPROUT_REQUIRED_FIELDS,
)


def get_sprout_package_errors(
properties: dict, check_required: bool
martonvago marked this conversation as resolved.
Show resolved Hide resolved
) -> list[CheckError]:
"""Checks the package `properties` against Sprout-specific requirements only.

Args:
properties: The package properties.
check_required: Whether the function should enforce the presence of required
fields.

Returns:
A list of errors. An empty list if no errors were found.
"""
errors = check_required_package_properties_not_blank(properties)
if check_required:
errors += check_fields_present(properties, PACKAGE_SPROUT_REQUIRED_FIELDS)
return errors
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import seedcase_sprout.core.checks as checks
from seedcase_sprout.core.sprout_checks.check_fields_not_blank import (
check_fields_not_blank,
)
from seedcase_sprout.core.sprout_checks.check_fields_present import (
check_fields_present,
)
from seedcase_sprout.core.sprout_checks.check_id_in_resource_path import (
check_id_in_resource_path,
)
from seedcase_sprout.core.sprout_checks.check_no_inline_data import check_no_inline_data
from seedcase_sprout.core.sprout_checks.check_resource_path_string import (
check_resource_path_string,
)
from seedcase_sprout.core.sprout_checks.required_fields import (
RESOURCE_SPROUT_REQUIRED_FIELDS,
)


def get_sprout_resource_errors(
properties: dict, check_required: bool, index: int | None = None
) -> list[checks.CheckError]:
"""Checks the resource `properties` against Sprout-specific requirements only.

Args:
properties: The resource properties.
check_required: Whether the function should enforce the presence of required
fields.
index: The index of the resource properties. Defaults to None.

Returns:
A list of errors. An empty list if no errors were found.
"""
errors = check_resource_path_string(properties, index)
errors += check_id_in_resource_path(properties, index)
errors += check_no_inline_data(properties, index)
errors += check_fields_not_blank(
properties,
RESOURCE_SPROUT_REQUIRED_FIELDS,
index,
)

if check_required:
errors += check_fields_present(
properties, RESOURCE_SPROUT_REQUIRED_FIELDS, index
)

return errors
86 changes: 86 additions & 0 deletions tests/core/sprout_checks/test_get_sprout_package_errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
from pytest import fixture, mark

from seedcase_sprout.core.properties import PackageProperties
from seedcase_sprout.core.sprout_checks.get_blank_value_for_type import (
get_blank_value_for_type,
)
from seedcase_sprout.core.sprout_checks.get_sprout_package_errors import (
get_sprout_package_errors,
)
from seedcase_sprout.core.sprout_checks.required_fields import (
PACKAGE_SPROUT_REQUIRED_FIELDS,
)


@fixture
def properties():
return PackageProperties(
name="package-1",
id="abc1",
title="Package 1",
description="A package.",
version="1.0.0",
created="2024-05-14T05:00:01+00:00",
licenses=[{"name": "a-license"}],
contributors=[{"title": "a contributor"}],
sources=[{"title": "a source"}],
).compact_dict


# Check required fields


def test_passes_full_package_properties(properties):
"""Should pass with a full set of package properties."""
assert get_sprout_package_errors(properties, check_required=True) == []


@mark.parametrize("name,type", PACKAGE_SPROUT_REQUIRED_FIELDS.items())
def test_error_found_if_fields_are_blank(properties, name, type):
"""Should find an error if there is one required field that is present but blank."""
properties[name] = get_blank_value_for_type(type)

errors = get_sprout_package_errors(properties, check_required=True)

assert len(errors) == 1
assert errors[0].json_path == f"$.{name}"
assert errors[0].validator == "blank"


@mark.parametrize("name", PACKAGE_SPROUT_REQUIRED_FIELDS.keys())
def test_error_found_if_required_fields_are_missing(properties, name):
"""Should find an error if there is a missing required field and required fields are
enforced."""
del properties[name]

errors = get_sprout_package_errors(properties, check_required=True)

assert len(errors) == 1
assert errors[0].json_path == f"$.{name}"
assert errors[0].validator == "required"


# Do not check required fields


def test_passes_full_package_properties_without_required_check(properties):
"""Should pass with a full set of package properties."""
assert get_sprout_package_errors(properties, check_required=False) == []


def test_passes_partial_package_properties_without_required_check():
"""Should pass with missing required fields when required fields are not
enforced."""
assert get_sprout_package_errors({}, check_required=False) == []


@mark.parametrize("name,type", PACKAGE_SPROUT_REQUIRED_FIELDS.items())
def test_error_found_if_fields_are_blank_without_required_check(properties, name, type):
"""Should find an error if there is one required field that is present but blank."""
properties[name] = get_blank_value_for_type(type)

errors = get_sprout_package_errors(properties, check_required=False)

assert len(errors) == 1
assert errors[0].json_path == f"$.{name}"
assert errors[0].validator == "blank"
123 changes: 123 additions & 0 deletions tests/core/sprout_checks/test_get_sprout_resource_errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
from pathlib import Path

from pytest import fixture, mark

from seedcase_sprout.core.properties import ResourceProperties
from seedcase_sprout.core.sprout_checks.get_blank_value_for_type import (
get_blank_value_for_type,
)
from seedcase_sprout.core.sprout_checks.get_json_path_to_resource_field import (
get_json_path_to_resource_field,
)
from seedcase_sprout.core.sprout_checks.get_sprout_resource_errors import (
get_sprout_resource_errors,
)
from seedcase_sprout.core.sprout_checks.required_fields import (
RESOURCE_SPROUT_REQUIRED_FIELDS,
)


@fixture
def properties():
return ResourceProperties(
name="resource-1",
path=str(Path("resources", "1", "data.parquet")),
title="Resource 1",
description="A resource.",
).compact_dict


@fixture
def properties_partial():
return ResourceProperties(
path=str(Path("resources", "1", "data.parquet")),
).compact_dict
signekb marked this conversation as resolved.
Show resolved Hide resolved


# Check required fields


def test_passes_full_resource_properties(properties):
"""Should pass with a full set of resource properties."""
signekb marked this conversation as resolved.
Show resolved Hide resolved
assert get_sprout_resource_errors(properties, check_required=True) == []


@mark.parametrize("index", [None, 2])
def test_error_found_if_inline_data_is_set(properties, index):
"""Should find an error if inline data is set."""
properties["data"] = "some data"

errors = get_sprout_resource_errors(properties, check_required=True, index=index)

assert len(errors) == 1
assert errors[0].json_path == get_json_path_to_resource_field("data", index)
assert errors[0].validator == "inline-data"


@mark.parametrize("index", [None, 2])
@mark.parametrize("name,type", RESOURCE_SPROUT_REQUIRED_FIELDS.items())
def test_error_found_if_fields_are_blank(properties, name, type, index):
"""Should find an error if there is one required field that is present but blank."""
properties[name] = get_blank_value_for_type(type)

errors = get_sprout_resource_errors(properties, check_required=True, index=index)
blank_errors = [error for error in errors if error.validator == "blank"]

assert len(blank_errors) == 1
assert blank_errors[0].json_path == get_json_path_to_resource_field(name, index)


@mark.parametrize("index", [None, 2])
@mark.parametrize("name", RESOURCE_SPROUT_REQUIRED_FIELDS.keys())
def test_error_found_if_required_fields_are_missing(properties, name, index):
"""Should find an error if there is a missing required field and required fields are
enforced."""
del properties[name]

errors = get_sprout_resource_errors(properties, check_required=True, index=index)
required_errors = [error for error in errors if error.validator == "required"]

assert len(required_errors) == 1
assert required_errors[0].json_path == get_json_path_to_resource_field(name, index)


# Do not check required fields


def test_passes_full_resource_properties_without_required_check(properties):
"""Should pass with a full set of resource properties."""
assert get_sprout_resource_errors(properties, check_required=False) == []


def test_passes_partial_resource_properties_without_required_check():
"""Should pass with missing required fields when required fields are not
enforced."""
assert get_sprout_resource_errors({}, check_required=False) == []


@mark.parametrize("path", ["", [], str(Path("resources", "1"))])
def test_error_found_if_data_path_is_incorrect_(properties, path):
"""Should find at least one error if `path` contains no resource ID, is not a
string, or is otherwise malformed."""
properties["path"] = path

errors = get_sprout_resource_errors(properties, check_required=False)

assert len(errors) >= 1


@mark.parametrize("index", [None, 2])
@mark.parametrize("name,type", RESOURCE_SPROUT_REQUIRED_FIELDS.items())
def test_error_found_if_fields_are_blank_without_required_check(
properties_partial, name, type, index
):
"""Should find an error if there is one required field that is present but blank."""
properties_partial[name] = get_blank_value_for_type(type)

errors = get_sprout_resource_errors(
properties_partial, check_required=False, index=index
)
blank_errors = [error for error in errors if error.validator == "blank"]

assert len(blank_errors) == 1
assert blank_errors[0].json_path == get_json_path_to_resource_field(name, index)
Loading