From 69bc08547f6e128cbbf8b7745523df2694183bf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Wed, 16 Nov 2022 01:58:39 -0600 Subject: [PATCH 1/3] feat: Support `patternProperties` --- .pre-commit-config.yaml | 8 +- docs/CONTRIBUTING.md | 12 ++ noxfile.py | 45 +++-- poetry.lock | 17 +- pyproject.toml | 4 + singer_sdk/typing.py | 21 +++ tests/conftest.py | 6 + tests/core/test_jsonschema_helpers.py | 156 +++++++++--------- .../jsonschema/additional_properties.json | 34 ++++ tests/snapshots/jsonschema/base.json | 29 ++++ tests/snapshots/jsonschema/duplicates.json | 29 ++++ .../duplicates_additional_properties.json | 34 ++++ .../jsonschema/pattern_properties.json | 36 ++++ tests/snapshots/jsonschema/required.json | 31 ++++ .../required_additional_properties.json | 36 ++++ .../jsonschema/required_duplicates.json | 32 ++++ ...ired_duplicates_additional_properties.json | 37 +++++ 17 files changed, 473 insertions(+), 94 deletions(-) create mode 100644 tests/snapshots/jsonschema/additional_properties.json create mode 100644 tests/snapshots/jsonschema/base.json create mode 100644 tests/snapshots/jsonschema/duplicates.json create mode 100644 tests/snapshots/jsonschema/duplicates_additional_properties.json create mode 100644 tests/snapshots/jsonschema/pattern_properties.json create mode 100644 tests/snapshots/jsonschema/required.json create mode 100644 tests/snapshots/jsonschema/required_additional_properties.json create mode 100644 tests/snapshots/jsonschema/required_duplicates.json create mode 100644 tests/snapshots/jsonschema/required_duplicates_additional_properties.json diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 70e0df23f..0aac1d4cf 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -20,7 +20,13 @@ repos: cookiecutter/.*/meltano.yml )$ - id: end-of-file-fixer - exclude: (cookiecutter/.*|docs/.*|samples/.*\.json) + exclude: | + (?x)^( + cookiecutter/.*| + docs/.*| + samples/.*\.json| + tests/snapshots/.*/.*\.json + )$ - id: trailing-whitespace exclude: | (?x)^( diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 9d55a2667..76fd35846 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -111,6 +111,18 @@ def test_windows_only(): Supported platform markers are `windows`, `darwin`, and `linux`. +### Snapshot Testing + +We use [pytest-snapshot](https://pypi.org/project/pytest-snapshot/) for snapshot testing. +To update snapshots, run: + +```bash +nox -rs update_snapshots +``` + +This will run all tests with the `snapshot` marker and update any snapshots that have changed. +Commit the updated snapshots to your branch if they are expected to change. + ## Testing Updates to Docs Documentation runs on Sphinx, using ReadtheDocs style template, and hosting from diff --git a/noxfile.py b/noxfile.py index 4ba8aeed6..e91f95846 100644 --- a/noxfile.py +++ b/noxfile.py @@ -28,6 +28,23 @@ "tests", "doctest", ) +test_dependencies = [ + "coverage[toml]", + "pytest", + "pytest-snapshot", + "freezegun", + "pandas", + "requests-mock", + # Cookiecutter tests + "black", + "cookiecutter", + "PyYAML", + "darglint", + "flake8", + "flake8-annotations", + "flake8-docstrings", + "mypy", +] @session(python=python_versions) @@ -53,22 +70,8 @@ def mypy(session: Session) -> None: def tests(session: Session) -> None: """Execute pytest tests and compute coverage.""" session.install(".") - session.install( - "coverage[toml]", - "pytest", - "freezegun", - "pandas", - "requests-mock", - # Cookiecutter tests - "black", - "cookiecutter", - "PyYAML", - "darglint", - "flake8", - "flake8-annotations", - "flake8-docstrings", - "mypy", - ) + session.install(*test_dependencies) + # temp fix until pyarrow is supported on python 3.11 if session.python != "3.11": session.install( @@ -91,6 +94,16 @@ def tests(session: Session) -> None: session.notify("coverage", posargs=[]) +@session(python=main_python_version) +def update_snapshots(session: Session) -> None: + """Update pytest snapshots.""" + args = session.posargs or ["-m", "snapshot"] + + session.install(".") + session.install(*test_dependencies) + session.run("pytest", "--snapshot-update", *args) + + @session(python=python_versions) def doctest(session: Session) -> None: """Run examples with xdoctest.""" diff --git a/poetry.lock b/poetry.lock index c8f446437..1a0385735 100644 --- a/poetry.lock +++ b/poetry.lock @@ -846,6 +846,17 @@ tomli = {version = ">=1.0.0", markers = "python_version < \"3.11\""} [package.extras] testing = ["argcomplete", "hypothesis (>=3.56)", "mock", "nose", "pygments (>=2.7.2)", "requests", "xmlschema"] +[[package]] +name = "pytest-snapshot" +version = "0.9.0" +description = "A plugin for snapshot testing with pytest." +category = "dev" +optional = false +python-versions = ">=3.5" + +[package.dependencies] +pytest = ">=3.0.0" + [[package]] name = "python-dateutil" version = "2.8.2" @@ -1338,7 +1349,7 @@ samples = [] [metadata] lock-version = "1.1" python-versions = "<3.12,>=3.7.1" -content-hash = "3c784adf4c987669d01ea83dab7eeb42133c68a8e862b47ace2d82bd871d7559" +content-hash = "45875f4451fa3f478d4943fa72081a068027dc1498109515137d2ff1a11071e4" [metadata.files] alabaster = [ @@ -1985,6 +1996,10 @@ pytest = [ {file = "pytest-7.2.0-py3-none-any.whl", hash = "sha256:892f933d339f068883b6fd5a459f03d85bfcb355e4981e146d2c7616c21fef71"}, {file = "pytest-7.2.0.tar.gz", hash = "sha256:c4014eb40e10f11f355ad4e3c2fb2c6c6d1919c73f3b5a433de4708202cade59"}, ] +pytest-snapshot = [ + {file = "pytest-snapshot-0.9.0.tar.gz", hash = "sha256:c7013c3abc3e860f9feff899f8b4debe3708650d8d8242a61bf2625ff64db7f3"}, + {file = "pytest_snapshot-0.9.0-py3-none-any.whl", hash = "sha256:4b9fe1c21c868fe53a545e4e3184d36bc1c88946e3f5c1d9dd676962a9b3d4ab"}, +] python-dateutil = [ {file = "python-dateutil-2.8.2.tar.gz", hash = "sha256:0123cacc1627ae19ddf3c27a5de5bd67ee4586fbdd6440d9748f8abb483d3e86"}, {file = "python_dateutil-2.8.2-py2.py3-none-any.whl", hash = "sha256:961d03dc3453ebbc59dbdea9e4e11c5651520a876d0f4db161e8674aae935da9"}, diff --git a/pyproject.toml b/pyproject.toml index 6f870d7be..701a420fa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -111,6 +111,9 @@ flake8 = "^3.9.0" flake8-annotations = "^2.9.1" flake8-docstrings = "^1.6.0" +[tool.poetry.group.dev.dependencies] +pytest-snapshot = "^0.9.0" + [tool.black] exclude = ".*simpleeval.*" @@ -125,6 +128,7 @@ addopts = '-vvv --ignore=singer_sdk/helpers/_simpleeval.py -m "not external"' markers = [ "external: Tests relying on external resources", "windows: Tests that only run on Windows", + "snapshot: Tests that use pytest-snapshot", ] [tool.commitizen] diff --git a/singer_sdk/typing.py b/singer_sdk/typing.py index 374c8734e..af5b14234 100644 --- a/singer_sdk/typing.py +++ b/singer_sdk/typing.py @@ -45,6 +45,7 @@ from __future__ import annotations +import json import sys from typing import Any, Generic, Mapping, TypeVar, Union, cast @@ -449,6 +450,7 @@ def __init__( self, *properties: Property, additional_properties: W | type[W] | None = None, + pattern_properties: dict[str, W | type[W]] | None = None, ) -> None: """Initialize ObjectType from its list of properties. @@ -456,9 +458,12 @@ def __init__( properties: Zero or more attributes for this JSON object. additional_properties: A schema to match against unnamed properties in this object. + pattern_properties: A dictionary of regex patterns to match against + property names, and the schema to match against the values. """ self.wrapped: list[Property] = list(properties) self.additional_properties = additional_properties + self.pattern_properties = pattern_properties @property def type_dict(self) -> dict: # type: ignore # OK: @classproperty vs @property @@ -481,8 +486,24 @@ def type_dict(self) -> dict: # type: ignore # OK: @classproperty vs @property if self.additional_properties: result["additionalProperties"] = self.additional_properties.type_dict + if self.pattern_properties: + result["patternProperties"] = { + k: v.type_dict for k, v in self.pattern_properties.items() + } + return result + def to_json(self, **kwargs: Any) -> str: + """Return a JSON string representation of the object. + + Args: + **kwargs: Additional keyword arguments to pass to `json.dumps`. + + Returns: + A JSON string. + """ + return json.dumps(self.type_dict, **kwargs) + class CustomType(JSONTypeHelper): """Accepts an arbitrary JSON Schema dictionary.""" diff --git a/tests/conftest.py b/tests/conftest.py index 5aa35f01a..4a88cd639 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -44,3 +44,9 @@ def outdir() -> str: yield name shutil.rmtree(name) + + +@pytest.fixture(scope="session") +def snapshot_dir() -> pathlib.Path: + """Return the path to the snapshot directory.""" + return pathlib.Path("tests/snapshots/") diff --git a/tests/core/test_jsonschema_helpers.py b/tests/core/test_jsonschema_helpers.py index a23ba5813..98a685da3 100644 --- a/tests/core/test_jsonschema_helpers.py +++ b/tests/core/test_jsonschema_helpers.py @@ -3,9 +3,11 @@ from __future__ import annotations import re -from typing import Callable, List +from pathlib import Path +from typing import Callable import pytest +from pytest_snapshot.plugin import Snapshot from singer_sdk.helpers._typing import ( JSONSCHEMA_ANNOTATION_SECRET, @@ -440,119 +442,121 @@ def test_array_type(): assert ArrayType(wrapped_type).type_dict == expected_json_schema +@pytest.mark.snapshot @pytest.mark.parametrize( - "properties,addtional_properties", + "schema_obj,snapshot_name", [ - ( - [ + pytest.param( + ObjectType( Property("id", StringType), Property("email", StringType), Property("username", StringType), Property("phone_number", StringType), - ], - None, + ), + "base.json", + id="no required, no duplicates, no additional properties", ), - ( - [ + pytest.param( + ObjectType( Property("id", StringType), Property("email", StringType), Property("username", StringType), Property("phone_number", StringType), - ], - StringType, + additional_properties=StringType, + ), + "additional_properties.json", + id="no required, no duplicates, additional properties", ), - ( - [ + pytest.param( + ObjectType( Property("id", StringType), Property("id", StringType), Property("email", StringType), Property("username", StringType), Property("phone_number", StringType), - ], - None, + ), + "duplicates.json", + id="no required, duplicates, no additional properties", ), - ( - [ + pytest.param( + ObjectType( Property("id", StringType), Property("id", StringType), Property("email", StringType), Property("username", StringType), Property("phone_number", StringType), - ], - StringType, + additional_properties=StringType, + ), + "duplicates_additional_properties.json", + id="no required, duplicates, additional properties", ), - ( - [ + pytest.param( + ObjectType( Property("id", StringType), - Property("email", StringType, True), - Property("username", StringType, True), + Property("email", StringType, required=True), + Property("username", StringType, required=True), Property("phone_number", StringType), - ], - None, + ), + "required.json", + id="required, no duplicates, no additional properties", ), - ( - [ + pytest.param( + ObjectType( Property("id", StringType), - Property("email", StringType, True), - Property("username", StringType, True), + Property("email", StringType, required=True), + Property("username", StringType, required=True), Property("phone_number", StringType), - ], - StringType, + additional_properties=StringType, + ), + "required_additional_properties.json", + id="required, no duplicates, additional properties", ), - ( - [ + pytest.param( + ObjectType( Property("id", StringType), - Property("email", StringType, True), - Property("email", StringType, True), - Property("username", StringType, True), + Property("email", StringType, required=True), + Property("email", StringType, required=True), + Property("username", StringType, required=True), Property("phone_number", StringType), - ], - None, + ), + "required_duplicates.json", + id="required, duplicates, no additional properties", ), - ( - [ + pytest.param( + ObjectType( Property("id", StringType), - Property("email", StringType, True), - Property("email", StringType, True), - Property("username", StringType, True), + Property("email", StringType, required=True), + Property("email", StringType, required=True), + Property("username", StringType, required=True), Property("phone_number", StringType), - ], - StringType, + additional_properties=StringType, + ), + "required_duplicates_additional_properties.json", + id="required, duplicates, additional properties", + ), + pytest.param( + ObjectType( + Property("id", StringType), + Property("email", StringType), + Property("username", StringType), + Property("phone_number", StringType), + pattern_properties={ + "^attr_[a-z]+$": StringType, + }, + ), + "pattern_properties.json", + id="pattern properties", ), - ], - ids=[ - "no requried, no duplicates, no additional properties", - "no requried, no duplicates, additional properties", - "no requried, duplicates, no additional properties", - "no requried, duplicates, additional properties", - "requried, no duplicates, no additional properties", - "requried, no duplicates, additional properties", - "requried, duplicates, no additional properties", - "requried, duplicates, additional properties", ], ) -def test_object_type(properties: list[Property], addtional_properties: JSONTypeHelper): - merged_property_schemas = { - name: schema for p in properties for name, schema in p.to_dict().items() - } - - required = [p.name for p in properties if not p.optional] - required_schema = {"required": required} if required else {} - addtional_properties_schema = ( - {"additionalProperties": addtional_properties.type_dict} - if addtional_properties - else {} - ) - - expected_json_schema = { - "type": "object", - "properties": merged_property_schemas, - **required_schema, - **addtional_properties_schema, - } - - object_type = ObjectType(*properties, additional_properties=addtional_properties) - assert object_type.type_dict == expected_json_schema +def test_object_type( + schema_obj: ObjectType, + snapshot_dir: Path, + snapshot_name: str, + snapshot: Snapshot, +): + snapshot.snapshot_dir = snapshot_dir.joinpath("jsonschema") + snapshot.assert_match(schema_obj.to_json(indent=2), snapshot_name) def test_custom_type(): diff --git a/tests/snapshots/jsonschema/additional_properties.json b/tests/snapshots/jsonschema/additional_properties.json new file mode 100644 index 000000000..a5e7aa5d3 --- /dev/null +++ b/tests/snapshots/jsonschema/additional_properties.json @@ -0,0 +1,34 @@ +{ + "type": "object", + "properties": { + "id": { + "type": [ + "string", + "null" + ] + }, + "email": { + "type": [ + "string", + "null" + ] + }, + "username": { + "type": [ + "string", + "null" + ] + }, + "phone_number": { + "type": [ + "string", + "null" + ] + } + }, + "additionalProperties": { + "type": [ + "string" + ] + } +} \ No newline at end of file diff --git a/tests/snapshots/jsonschema/base.json b/tests/snapshots/jsonschema/base.json new file mode 100644 index 000000000..771725769 --- /dev/null +++ b/tests/snapshots/jsonschema/base.json @@ -0,0 +1,29 @@ +{ + "type": "object", + "properties": { + "id": { + "type": [ + "string", + "null" + ] + }, + "email": { + "type": [ + "string", + "null" + ] + }, + "username": { + "type": [ + "string", + "null" + ] + }, + "phone_number": { + "type": [ + "string", + "null" + ] + } + } +} \ No newline at end of file diff --git a/tests/snapshots/jsonschema/duplicates.json b/tests/snapshots/jsonschema/duplicates.json new file mode 100644 index 000000000..771725769 --- /dev/null +++ b/tests/snapshots/jsonschema/duplicates.json @@ -0,0 +1,29 @@ +{ + "type": "object", + "properties": { + "id": { + "type": [ + "string", + "null" + ] + }, + "email": { + "type": [ + "string", + "null" + ] + }, + "username": { + "type": [ + "string", + "null" + ] + }, + "phone_number": { + "type": [ + "string", + "null" + ] + } + } +} \ No newline at end of file diff --git a/tests/snapshots/jsonschema/duplicates_additional_properties.json b/tests/snapshots/jsonschema/duplicates_additional_properties.json new file mode 100644 index 000000000..a5e7aa5d3 --- /dev/null +++ b/tests/snapshots/jsonschema/duplicates_additional_properties.json @@ -0,0 +1,34 @@ +{ + "type": "object", + "properties": { + "id": { + "type": [ + "string", + "null" + ] + }, + "email": { + "type": [ + "string", + "null" + ] + }, + "username": { + "type": [ + "string", + "null" + ] + }, + "phone_number": { + "type": [ + "string", + "null" + ] + } + }, + "additionalProperties": { + "type": [ + "string" + ] + } +} \ No newline at end of file diff --git a/tests/snapshots/jsonschema/pattern_properties.json b/tests/snapshots/jsonschema/pattern_properties.json new file mode 100644 index 000000000..aab4147b4 --- /dev/null +++ b/tests/snapshots/jsonschema/pattern_properties.json @@ -0,0 +1,36 @@ +{ + "type": "object", + "properties": { + "id": { + "type": [ + "string", + "null" + ] + }, + "email": { + "type": [ + "string", + "null" + ] + }, + "username": { + "type": [ + "string", + "null" + ] + }, + "phone_number": { + "type": [ + "string", + "null" + ] + } + }, + "patternProperties": { + "^attr_[a-z]+$": { + "type": [ + "string" + ] + } + } +} \ No newline at end of file diff --git a/tests/snapshots/jsonschema/required.json b/tests/snapshots/jsonschema/required.json new file mode 100644 index 000000000..5484f5247 --- /dev/null +++ b/tests/snapshots/jsonschema/required.json @@ -0,0 +1,31 @@ +{ + "type": "object", + "properties": { + "id": { + "type": [ + "string", + "null" + ] + }, + "email": { + "type": [ + "string" + ] + }, + "username": { + "type": [ + "string" + ] + }, + "phone_number": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "email", + "username" + ] +} \ No newline at end of file diff --git a/tests/snapshots/jsonschema/required_additional_properties.json b/tests/snapshots/jsonschema/required_additional_properties.json new file mode 100644 index 000000000..17a773ded --- /dev/null +++ b/tests/snapshots/jsonschema/required_additional_properties.json @@ -0,0 +1,36 @@ +{ + "type": "object", + "properties": { + "id": { + "type": [ + "string", + "null" + ] + }, + "email": { + "type": [ + "string" + ] + }, + "username": { + "type": [ + "string" + ] + }, + "phone_number": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "email", + "username" + ], + "additionalProperties": { + "type": [ + "string" + ] + } +} \ No newline at end of file diff --git a/tests/snapshots/jsonschema/required_duplicates.json b/tests/snapshots/jsonschema/required_duplicates.json new file mode 100644 index 000000000..aaa890002 --- /dev/null +++ b/tests/snapshots/jsonschema/required_duplicates.json @@ -0,0 +1,32 @@ +{ + "type": "object", + "properties": { + "id": { + "type": [ + "string", + "null" + ] + }, + "email": { + "type": [ + "string" + ] + }, + "username": { + "type": [ + "string" + ] + }, + "phone_number": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "email", + "email", + "username" + ] +} \ No newline at end of file diff --git a/tests/snapshots/jsonschema/required_duplicates_additional_properties.json b/tests/snapshots/jsonschema/required_duplicates_additional_properties.json new file mode 100644 index 000000000..c433255b1 --- /dev/null +++ b/tests/snapshots/jsonschema/required_duplicates_additional_properties.json @@ -0,0 +1,37 @@ +{ + "type": "object", + "properties": { + "id": { + "type": [ + "string", + "null" + ] + }, + "email": { + "type": [ + "string" + ] + }, + "username": { + "type": [ + "string" + ] + }, + "phone_number": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "email", + "email", + "username" + ], + "additionalProperties": { + "type": [ + "string" + ] + } +} \ No newline at end of file From 49174b42058ddd967a1c7033a5edd2053800566d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Wed, 16 Nov 2022 15:27:18 -0600 Subject: [PATCH 2/3] Add `__all__` to _singerlib --- singer_sdk/_singerlib/__init__.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/singer_sdk/_singerlib/__init__.py b/singer_sdk/_singerlib/__init__.py index 76e039d92..368ca191a 100644 --- a/singer_sdk/_singerlib/__init__.py +++ b/singer_sdk/_singerlib/__init__.py @@ -17,3 +17,22 @@ write_message, ) from singer_sdk._singerlib.schema import Schema, resolve_schema_references + +__all__ = [ + "Catalog", + "CatalogEntry", + "Metadata", + "MetadataMapping", + "SelectionMask", + "StreamMetadata", + "ActivateVersionMessage", + "Message", + "RecordMessage", + "SchemaMessage", + "SingerMessageType", + "StateMessage", + "exclude_null_dict", + "write_message", + "Schema", + "resolve_schema_references", +] From acc38f39ed35a5c0471bb07cf238202bbbbe7cae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Rami=CC=81rez=20Mondrago=CC=81n?= Date: Wed, 16 Nov 2022 15:27:58 -0600 Subject: [PATCH 3/3] Use more generic `Mapping` annotation --- singer_sdk/typing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/singer_sdk/typing.py b/singer_sdk/typing.py index af5b14234..5d38f0abe 100644 --- a/singer_sdk/typing.py +++ b/singer_sdk/typing.py @@ -450,7 +450,7 @@ def __init__( self, *properties: Property, additional_properties: W | type[W] | None = None, - pattern_properties: dict[str, W | type[W]] | None = None, + pattern_properties: Mapping[str, W | type[W]] | None = None, ) -> None: """Initialize ObjectType from its list of properties.