adding matcher POC #761

valkolovos · 2024-08-13T15:18:44Z

matcher POC

I just want to verify if this is headed in the right direction

valkolovos · 2024-08-13T15:21:26Z

examples/tests/v3/basic_flask_server.py

This is just to serve the expected response and would need to be heavily modified for a final product.

valkolovos · 2024-08-13T15:22:26Z

src/pact/v3/matchers/matchers.py

I implemented a generator option only in the regex matcher again just as a POC.

The Matcher class will (I think) be useful when implementing more complex logic, just as being able to "OR" together different matchers. As far as I can tell, there's no way to do the "OR" logic directly in parameter JSON (e.g. { "attribute": { "value": "expected", "pact:matcher:type": "regex", "regex": "expect.*" } }.

JP-Ellis

Love the PoC!

I have a few minor nitpicks, but otherwise this looks like an amazing start!

Let me know how you would like me to support you in this going forward 🚀

JP-Ellis · 2024-08-21T01:04:56Z

examples/tests/v3/test_matchers.py

+from pathlib import Path
+
+from examples.tests.v3.basic_flask_server import start_provider
+from pact.v3 import Pact, Verifier, matchers


I would want to avoid polluting the namespace, so I agree with your choice of importing a module and calling matchers.like and matchers.regex etc.

My nit here is that I would want to name it match so that the syntax is match.regex(...) and match.like(...), etc. This way, it reads a bit more like English :)

I feel like the module that defines the actual classes (Matcher, MatcherEncoder, etc) should be called "matchers" since they are objects. I'd be happy to create a module called match and move the functions that return those matchers into that module.

Just started going through your updated PR. I'm still thinking that we should expose the the functions behind a common match.{func} interface, as I do think it becomes most intuitive. It also serves as a nice way to distinguish match.{func} from similar generate.{func}.

I also agree that having matchers as the module name for the Matcher classes makes sense. Having said that, I don't think people would often (or ever?) need to interact with the class directly. So I'm liking what you've done whereby the matchers are defined in a submodule, and functions re-exported at a higher level.

JP-Ellis · 2024-08-21T01:08:03Z

src/pact/v3/interaction/_base.py

+        if not isinstance(body, str):
+            body_str = json.dumps(body, cls=MatcherEncoder)


Nit: Avoid negative isinstance assertions.

If Python was strongly typed, this wouldn't matter; however, the type annotations are merely hint and at runtime, end users can in fact input whatever they want. So we need to handle cases of with_body(MyClassWithouSerialization(...)) (which your if statement would catch).

JP-Ellis · 2024-09-19T07:01:26Z

I've started working on the PR. Pushed a few changes (which are so far relatively minor).

I do anticipate making some larger changes at a later date, but I just want to get a feel for it first :)

The `dict`, `list` and `tuple` types require argument. Also, the use of the lowercase variants is not compatible with Python 3.8 (which will be dropped soon anyway... but still need to support it for a little while longer). Signed-off-by: JP-Ellis <[email protected]>

The `JSONEncoder` class uses `o` as the argument and does not enforce a positional argument. This means we need to support the possible use of `default(o=...)`. Signed-off-by: JP-Ellis <[email protected]>

The type annotation that goes alongside a `**kwargs` types the _values_. Therefore a `**kwargs: Foo` will result in `kwargs` being of type `dict[str, Foo]`. Signed-off-by: JP-Ellis <[email protected]>

While equivalent, Optional and Union quickly can become quite verbose and more difficult to parse. Compare ``` Optional[Union[str, bool]] ``` to ``` str | bool | None ``` Signed-off-by: JP-Ellis <[email protected]>

They are functionally equivalent, but with ABC being made to be more intuitive than the use of metaclass Signed-off-by: JP-Ellis <[email protected]>

Signed-off-by: JP-Ellis <[email protected]>

In particular, splitting up the class, functions and types into separate modules. Signed-off-by: JP-Ellis <[email protected]>

codecov · 2024-09-20T13:46:09Z

Codecov Report

Attention: Patch coverage is 73.40153% with 104 lines in your changes missing coverage. Please review.

Project coverage is 78%. Comparing base (2845821) to head (a450209).
Report is 50 commits behind head on master.

Files with missing lines	Patch %	Lines
src/pact/v3/match/__init__.py	65%	58 Missing ⚠️
src/pact/v3/generate/__init__.py	61%	30 Missing ⚠️
src/pact/v3/match/matcher.py	81%	11 Missing ⚠️
src/pact/v3/util.py	90%	4 Missing ⚠️
src/pact/v3/generate/generator.py	92%	1 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##           master   #761    +/-   ##
======================================
- Coverage      78%    78%    -1%     
======================================
  Files          23     29     +6     
  Lines        2673   3060   +387     
======================================
+ Hits         2111   2394   +283     
- Misses        562    666   +104

Flag	Coverage Δ
examples	`60% <71%> (+2%)`	⬆️
tests	`74% <51%> (-4%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: JP-Ellis <[email protected]>

Strictly speaking, the typing annotation would allow for arrays to be used as keys in dictionaries, but as these are 'merely' typing hints, this should not matter much and should rarely lead to confusion. Instead, this will simplify some typing in cases dealing with arrays and mappings. Signed-off-by: JP-Ellis <[email protected]>

A massive thanks to Val from all of the groundwork! I'm playing around here with shadowing Python built-ins to provide a more Pythonic interface. E.g., `match.int` and `match.str`, etc. I've also adjusted a number of arguments to allow shadowing built-ins (e.g., min, max) as the limited context poses little risk of confusion. Lastly, restructed a number of the argument to ensure that the value is _always_ a position argument, and in most cases, the remaining arguments are _always_ key word arguments. This will prevent `match.int(1, 2, 3)` where it may not be immediately obvious what each argument corresponds to. Signed-off-by: JP-Ellis <[email protected]>

Signed-off-by: JP-Ellis <[email protected]>

The Pact ecosystem has a number of commonly used names for matchers. Aliases are added to ensure compatibility with what one might expect from using another Pact implementation. Signed-off-by: JP-Ellis <[email protected]>

Signed-off-by: JP-Ellis <[email protected]>

Instead of having multiple types sub-modules for the generators and matchers, create a single shared `pact.v3.types` module. Signed-off-by: JP-Ellis <[email protected]>

Signed-off-by: JP-Ellis <[email protected]>

The generator and matcher behave very similarly, and this brings the generator in line with the matcher declaration. Signed-off-by: JP-Ellis <[email protected]>

As much as it would be nice to restrict a type var to only matchable types, there seems to be an issue when combining typevars with mappings/sequences. So instead, I'm removing `MatchableT` and will re-instate unrestricted `_T` types in the modules. Signed-off-by: JP-Ellis <[email protected]>

Signed-off-by: JP-Ellis <[email protected]>

This brings the generators broadly in line with the matchers, including the shadowing of Python builtins to allow for `generate.int()` and `generate.float`, etc. Signed-off-by: JP-Ellis <[email protected]>

This incorporates a couple of recent changes in the recent of the codebase: - The addition of EachKeyMatcher and EachValueMatcher - The refactor of the generate module - The removal of the MatchableT TypeVar Signed-off-by: JP-Ellis <[email protected]>

Signed-off-by: JP-Ellis <[email protected]>

valkolovos · 2024-10-01T14:21:57Z

src/pact/v3/generate/generator.py

+        integration_fields: Mapping[str, Any] | None = None,
+        generator_json_fields: Mapping[str, Any] | None = None,


Is it possible to provide examples of how these two might be used?

Yeah, good call.

The reasoning initially is that the way the generators/matchers are serialised into the 'integration JSON' format is slightly different to the way they are serialised when passed directly to the matching_rules or generators commands. E.g., in one, you have to use "pact:matcher:type": "..." whereas the other you don't.

As far as I know, all the other fields are the same, but if there were others that had to be differentiated, then this is there.

Thinking back on it though, I'm concerned that this might be a case of premature generalisation.

Signed-off-by: JP-Ellis <[email protected]>

A generator is only really useful if a value is _not_ provided. Otherwise, the generator is likely to cause confusion when the test doesn't generator the expected result. Signed-off-by: JP-Ellis <[email protected]>

Signed-off-by: JP-Ellis <[email protected]>

The ability to add additional fields to either the integration JSON or FFI JSON representations is not used, and may never be needed. Signed-off-by: JP-Ellis <[email protected]>

JP-Ellis · 2024-10-02T04:18:28Z

Even though this is failing on Python 3.8, I will still merge this as Python 3.8 is being dropped imminently (#796)

valkolovos commented Aug 13, 2024

View reviewed changes

valkolovos force-pushed the matcher_poc branch 5 times, most recently from 84dbcae to e5939c3 Compare August 13, 2024 21:41

JP-Ellis reviewed Aug 21, 2024

View reviewed changes

valkolovos force-pushed the matcher_poc branch 3 times, most recently from 4a97348 to a159b47 Compare September 17, 2024 15:22

valkolovos marked this pull request as ready for review September 17, 2024 15:53

valkolovos and others added 13 commits September 20, 2024 07:39

adding matcher POC

e77b562

Matcher / generator implementations and examples

18d0e12

linting and python version fixes

a843538

linting and formatting fixes

1ca2832

fix: incompatible override

834c73b

The `JSONEncoder` class uses `o` as the argument and does not enforce a positional argument. This means we need to support the possible use of `default(o=...)`. Signed-off-by: JP-Ellis <[email protected]>

fix: kwargs typing

2902425

The type annotation that goes alongside a `**kwargs` types the _values_. Therefore a `**kwargs: Foo` will result in `kwargs` being of type `dict[str, Foo]`. Signed-off-by: JP-Ellis <[email protected]>

refactor: prefer | over Optional and Union

bedd5a8

While equivalent, Optional and Union quickly can become quite verbose and more difficult to parse. Compare ``` Optional[Union[str, bool]] ``` to ``` str | bool | None ``` Signed-off-by: JP-Ellis <[email protected]>

chore: prefer ABC over ABCMeta

7830611

They are functionally equivalent, but with ABC being made to be more intuitive than the use of metaclass Signed-off-by: JP-Ellis <[email protected]>

docs: add matcher module preamble

ff434a0

Signed-off-by: JP-Ellis <[email protected]>

refactor: rename matchers to match

85d2fea

Signed-off-by: JP-Ellis <[email protected]>

chore: re-organise match module

511c41a

In particular, splitting up the class, functions and types into separate modules. Signed-off-by: JP-Ellis <[email protected]>

fix recursive typing issues

4cd67f5

valkolovos force-pushed the matcher_poc branch from c8baa3c to 4cd67f5 Compare September 20, 2024 13:39

fix issues importing MatcherEncoder

fe4d4af

fixing typing issues for python3.9

547fbe1

JP-Ellis added 4 commits September 30, 2024 18:21

chore: split stdlib and 3rd party types

370aa3a

Signed-off-by: JP-Ellis <[email protected]>

docs: add module docstring

b44a238

Signed-off-by: JP-Ellis <[email protected]>

refactor: rename generators to generate

9662008

Signed-off-by: JP-Ellis <[email protected]>

feat: add strftime to java date format converter

57aeb3b

Signed-off-by: JP-Ellis <[email protected]>

JP-Ellis force-pushed the matcher_poc branch from acf1710 to 57aeb3b Compare September 30, 2024 08:35

JP-Ellis added 17 commits September 30, 2024 19:01

chore: add pyi to editor config

77eeeab

Signed-off-by: JP-Ellis <[email protected]>

chore: add test for full ISO 8601 date

dc5acc2

Signed-off-by: JP-Ellis <[email protected]>

feat: add match aliases

962f642

The Pact ecosystem has a number of commonly used names for matchers. Aliases are added to ensure compatibility with what one might expect from using another Pact implementation. Signed-off-by: JP-Ellis <[email protected]>

feat: add uuid matcher

0856c4e

Signed-off-by: JP-Ellis <[email protected]>

refactor: generate module in style of match module

2de9b8f

Signed-off-by: JP-Ellis <[email protected]>

refactor: create pact.v3.types module

f0307b9

Instead of having multiple types sub-modules for the generators and matchers, create a single shared `pact.v3.types` module. Signed-off-by: JP-Ellis <[email protected]>

chore: minor improvements to match.matcher

289fd5e

Signed-off-by: JP-Ellis <[email protected]>

chore: align generator with matcher

fabd01d

The generator and matcher behave very similarly, and this brings the generator in line with the matcher declaration. Signed-off-by: JP-Ellis <[email protected]>

feat: add each key/value matchers

d8b1f3f

Signed-off-by: JP-Ellis <[email protected]>

refactor: generators module

a00267f

This brings the generators broadly in line with the matchers, including the shadowing of Python builtins to allow for `generate.int()` and `generate.float`, etc. Signed-off-by: JP-Ellis <[email protected]>

refactor: match module

5f03481

This incorporates a couple of recent changes in the recent of the codebase: - The addition of EachKeyMatcher and EachValueMatcher - The refactor of the generate module - The removal of the MatchableT TypeVar Signed-off-by: JP-Ellis <[email protected]>

chore: get test to run again

1f2fb0b

Signed-off-by: JP-Ellis <[email protected]>

feat: add ArrayContainsMatcher

4950f50

Signed-off-by: JP-Ellis <[email protected]>

chore: add boolean alias

9814b3d

Signed-off-by: JP-Ellis <[email protected]>

valkolovos commented Oct 1, 2024

View reviewed changes

JP-Ellis added 4 commits October 2, 2024 11:08

chore: fix compatibility with Python <= 3.9

1c6170c

Signed-off-by: JP-Ellis <[email protected]>

fix: ensure matchers optionally use generators

ba8c3c8

A generator is only really useful if a value is _not_ provided. Otherwise, the generator is likely to cause confusion when the test doesn't generator the expected result. Signed-off-by: JP-Ellis <[email protected]>

chore: fix match tests

57d4405

Signed-off-by: JP-Ellis <[email protected]>

chore: remove unnused generalisation

a450209

The ability to add additional fields to either the integration JSON or FFI JSON representations is not used, and may never be needed. Signed-off-by: JP-Ellis <[email protected]>

JP-Ellis merged commit 909a04e into pact-foundation:master Oct 2, 2024
29 of 35 checks passed

valkolovos deleted the matcher_poc branch October 2, 2024 17:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding matcher POC #761

adding matcher POC #761

valkolovos commented Aug 13, 2024

valkolovos Aug 13, 2024

valkolovos Aug 13, 2024

valkolovos Aug 13, 2024 •

edited

Loading

JP-Ellis left a comment

JP-Ellis Aug 21, 2024

valkolovos Sep 17, 2024

JP-Ellis Sep 19, 2024

JP-Ellis Aug 21, 2024

JP-Ellis commented Sep 19, 2024

codecov bot commented Sep 20, 2024 •

edited

Loading

valkolovos Oct 1, 2024

JP-Ellis Oct 2, 2024

JP-Ellis commented Oct 2, 2024

		if not isinstance(body, str):
		body_str = json.dumps(body, cls=MatcherEncoder)

		integration_fields: Mapping[str, Any] \| None = None,
		generator_json_fields: Mapping[str, Any] \| None = None,

adding matcher POC #761

adding matcher POC #761

Conversation

valkolovos commented Aug 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valkolovos Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

JP-Ellis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JP-Ellis commented Sep 19, 2024

codecov bot commented Sep 20, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JP-Ellis commented Oct 2, 2024

valkolovos Aug 13, 2024 •

edited

Loading

codecov bot commented Sep 20, 2024 •

edited

Loading