feat: add array contains matcher #518

MaxCampman · 2024-09-07T01:28:01Z

What?

Add array-contains matcher (e.g. JS version docs).
I was able to use this matcher via a workaround but thought I should add it here to make it available to others.
This was added to the pact-reference project here: feat: allow non-object matchers for array-contains pact-reference#459
In order to incorporate this change, I have had to update the pact-ffi version to 0.4.23 (from 0.4.16)
This required some small tweaks to the integration tests and some tweaking of the script used to fetch the libraries (there were a few name changes for libraries).
Actual change is implemented as:
- Addition of: src/PactNet.Abstractions/Matchers/ArrayContainsMatcher.cs
- Addition to: src/PactNet.Abstractions/Matchers/Match.cs
- Addition to: src/PactNet.Abstractions/Matchers/MatcherConverter.cs

NOTE:

I unfortunately do not have access to an Apple computer at the moment.
This likely means that the build script needs some further tweaking for the OSX-specific script (if [[ "$OSTYPE" == "darwin"* ]]; then)

adamrodger

The FFI change should probably be split into a separate preceding change just because of the extra complexity around the breaking changes in the FFI packaging. I could see that causing issues so a dedicated change would be better.

In terms of the example, the pact spec describes this matcher as checking that all variants are contained within a collection, but this example doesn't really reflect that given the collection is only of one type and thus only one variant. The eachLike matcher is better in that scenario. I think the example should somehow contain different types (and if it's too difficult to clearly do that then perhaps the matcher doesn't make as much sense in a typed language Vs something like JavaScript).

I've not reviewed the integration tests as I'm on my phone and they're quite big, so there may be further comments there later.

Also check any line endings settings in your editor because a few places have weird extra blank lines and things, plus obviously the sln file shows a huge diff when there probably isn't one.

adamrodger · 2024-09-07T18:52:29Z

PactNet.sln

-		SolutionGuid = {C2CBC30C-92D4-4E3A-A5B8-1E5D4E938DFC}
-	EndGlobalSection
-EndGlobal
+


issue: Something appears to have gone weird with this file, perhaps line endings? Iirc sln files must have CRLF line endings always, even on non-Windows systems

The gitignore file also may have this issue because the final line shows a change when there isn't one

adamrodger · 2024-09-07T18:57:12Z

samples/OrdersApi/Consumer.Tests/pacts/Fulfilment API-Orders API.json

+                          "combine": "AND",
+                          "matchers": [
+                            {
+                              "match": "type"


issue: This matching rule looks wrong, should it be a date matcher instead?

This is from this line:

Date = Match.Type(expected1.Date.ToString("O"))

So is indeed meant to be type

adamrodger · 2024-09-07T18:59:12Z

samples/OrdersApi/Consumer.Tests/pacts/Fulfilment API-Orders API.json

@@ -88,6 +208,126 @@
      },
      "type": "Synchronous/HTTP"
    },
+    {
+      "description": "a request for multiple orders by id",


issue: This interaction appears to be in here twice. I was hoping the updated FFI would fix this issue, but perhaps it doesn't

This is my fault, can see the description here is different to the one above.
Will amend

adamrodger · 2024-09-07T19:02:22Z

src/PactNet.Abstractions/Matchers/ArrayContainsMatcher.cs

@@ -0,0 +1,17 @@
+using System.Text.Json.Serialization;
+
+namespace PactNet.Matchers;


style: The project uses namespace blocks instead of file scoped namespaces

Noted, will amend

adamrodger · 2024-09-07T19:02:45Z

src/PactNet.Abstractions/Matchers/ArrayContainsMatcher.cs

+
+namespace PactNet.Matchers;
+
+public class ArrayContainsMatcher : IMatcher


issue: The public API should be documented

Noted, will amend

adamrodger · 2024-09-07T19:03:41Z

src/PactNet.Abstractions/Matchers/Match.cs

@@ -168,5 +168,10 @@ public static IMatcher Include(string example)
        {
            return new IncludeMatcher(example);
        }
+
+        public static IMatcher ArrayContains(dynamic[] variations)


issue: The public API should be documented

Noted, will amend

This one still needs documenting

adamrodger · 2024-09-07T19:05:06Z

tests/PactNet.Abstractions.Tests/Matchers/ArrayContainsMatcherTests.cs

+            var actual = JsonSerializer.Serialize(matcher);
+
+            // Assert
+            actual.Should().Be(@"{""pact:matcher:type"":""array-contains"",""variants"":[""Thing1"",""Thing2""]}");


suggestion: You can use new string literals to format the JSON nicely here

Thanks, quite the oversight from me

Nevermind: ArrayContainsMatcherTests.cs(27, 17): [CS8370] Feature 'raw string literals' is not available in C# 7.3. Please use language version 11.0 or greater.

Ah, yes my bad. These have to support .Net Framework also

MaxCampman

Thanks for taking the time to go through this, especially on a weekend.

I will open a new MR to just update the FFI, and will come back to this one to make the requested changes

MaxCampman · 2024-09-07T21:16:41Z

samples/OrdersApi/Consumer.Tests/pacts/Fulfilment API-Orders API.json

+                          "combine": "AND",
+                          "matchers": [
+                            {
+                              "match": "type"


This is from this line:

Date = Match.Type(expected1.Date.ToString("O"))

So is indeed meant to be type

MaxCampman · 2024-09-07T21:18:39Z

samples/OrdersApi/Consumer.Tests/pacts/Fulfilment API-Orders API.json

@@ -88,6 +208,126 @@
      },
      "type": "Synchronous/HTTP"
    },
+    {
+      "description": "a request for multiple orders by id",


This is my fault, can see the description here is different to the one above.
Will amend

MaxCampman · 2024-09-07T21:18:51Z

src/PactNet.Abstractions/Matchers/ArrayContainsMatcher.cs

@@ -0,0 +1,17 @@
+using System.Text.Json.Serialization;
+
+namespace PactNet.Matchers;


Noted, will amend

MaxCampman · 2024-09-07T21:19:09Z

src/PactNet.Abstractions/Matchers/ArrayContainsMatcher.cs

+
+namespace PactNet.Matchers;
+
+public class ArrayContainsMatcher : IMatcher


Noted, will amend

MaxCampman · 2024-09-07T21:19:18Z

src/PactNet.Abstractions/Matchers/Match.cs

@@ -168,5 +168,10 @@ public static IMatcher Include(string example)
        {
            return new IncludeMatcher(example);
        }
+
+        public static IMatcher ArrayContains(dynamic[] variations)


Noted, will amend

MaxCampman · 2024-09-07T21:19:50Z

tests/PactNet.Abstractions.Tests/Matchers/ArrayContainsMatcherTests.cs

+            var actual = JsonSerializer.Serialize(matcher);
+
+            // Assert
+            actual.Should().Be(@"{""pact:matcher:type"":""array-contains"",""variants"":[""Thing1"",""Thing2""]}");


Thanks, quite the oversight from me

Nevermind: ArrayContainsMatcherTests.cs(27, 17): [CS8370] Feature 'raw string literals' is not available in C# 7.3. Please use language version 11.0 or greater.

adamrodger · 2024-09-08T13:21:21Z

@MaxCampman if you rebase onto latest then the FFI upgrade should already be there 👍

adamrodger · 2024-10-06T09:18:17Z

samples/OrdersApi/Consumer.Tests/OrdersClientTests.cs

+                .WithStatus(HttpStatusCode.OK)
+                .WithJsonBody(Match.ArrayContains(new dynamic[]
+                {
+                    new


issue: I think for this example to 'come to life' then we'd really need it to match multiple different possible variants

Perhaps if there were different order types or something so that they could have different fields on them, then the matching rules could reflect those variants. Like a fulfilled order has an extra date on it for when it was completed, or something like that.

adamrodger · 2024-10-06T09:19:29Z

samples/OrdersApi/Provider.Tests/ProviderStateMiddleware.cs

@@ -52,6 +54,15 @@ private async Task EnsureEventExistsAsync(IDictionary<string, object> parameters
            await this.orders.InsertAsync(new OrderDto(id.GetInt32(), OrderStatus.Fulfilling, DateTimeOffset.Now));
        }

+        private async Task EnsureEventsExistAsync(IDictionary<string, object> parameters)


docs: Needs doc string like other methods

adamrodger · 2024-10-06T09:20:11Z

samples/OrdersApi/Provider/Orders/OrdersController.cs

@@ -46,6 +47,29 @@ public async Task<IActionResult> GetByIdAsync(int id)
            }
        }

+        [HttpGet("many/{ids}", Name = "getMany")]


docs: Needs doc string like other methods

adamrodger · 2024-10-06T09:20:46Z

src/PactNet.Abstractions/Matchers/ArrayContainsMatcher.cs

+
+namespace PactNet.Matchers
+{
+    public class ArrayContainsMatcher : IMatcher


docs: Needs doc string like other matchers

adamrodger · 2024-10-06T09:21:37Z

src/PactNet.Abstractions/Matchers/Match.cs

@@ -168,5 +168,10 @@ public static IMatcher Include(string example)
        {
            return new IncludeMatcher(example);
        }
+
+        public static IMatcher ArrayContains(dynamic[] variations)


This one still needs documenting

adamrodger · 2024-10-06T09:22:38Z

tests/PactNet.Abstractions.Tests/Matchers/ArrayContainsMatcherTests.cs

+            var actual = JsonSerializer.Serialize(matcher);
+
+            // Assert
+            actual.Should().Be(@"{""pact:matcher:type"":""array-contains"",""variants"":[""Thing1"",""Thing2""]}");


Ah, yes my bad. These have to support .Net Framework also

adamrodger · 2024-10-06T09:24:00Z

tests/PactNet.Tests/Matchers/ArrayContainsMatcherTests.cs

+            .WithStatus(HttpStatusCode.OK)
+            .WithJsonBody(Match.ArrayContains([
+                Match.Regex("Thing 1", "Thing [0-9]+"),
+                Match.Regex("Thing X", "Thing [A-Z]+"),


comment: This is a good test because it shows using different variants, which we should replicate in the example usage

adamrodger requested changes Sep 7, 2024

View reviewed changes

MaxCampman commented Sep 7, 2024

View reviewed changes

MaxCampman force-pushed the feat-add-array-contains-matcher branch 3 times, most recently from 1915b96 to ff6f37c Compare September 8, 2024 15:18

feat: add array contains matcher

b03a9ea

MaxCampman force-pushed the feat-add-array-contains-matcher branch from ff6f37c to b03a9ea Compare September 8, 2024 15:19

adamrodger requested changes Oct 6, 2024

View reviewed changes

adamrodger added this to the 5.1.0 milestone Oct 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add array contains matcher #518

feat: add array contains matcher #518

MaxCampman commented Sep 7, 2024

adamrodger left a comment

adamrodger Sep 7, 2024

adamrodger Sep 7, 2024

MaxCampman Sep 7, 2024

adamrodger Sep 7, 2024

MaxCampman Sep 7, 2024

MaxCampman Sep 8, 2024

adamrodger Sep 7, 2024

MaxCampman Sep 7, 2024

adamrodger Sep 7, 2024

MaxCampman Sep 7, 2024

adamrodger Sep 7, 2024

MaxCampman Sep 7, 2024

adamrodger Oct 6, 2024

adamrodger Sep 7, 2024

MaxCampman Sep 7, 2024 •

edited

Loading

adamrodger Oct 6, 2024

MaxCampman left a comment

MaxCampman Sep 7, 2024

MaxCampman Sep 7, 2024

MaxCampman Sep 7, 2024

MaxCampman Sep 7, 2024

MaxCampman Sep 7, 2024

MaxCampman Sep 7, 2024 •

edited

Loading

adamrodger commented Sep 8, 2024

adamrodger Oct 6, 2024

adamrodger Oct 6, 2024

adamrodger Oct 6, 2024

adamrodger Oct 6, 2024

adamrodger Oct 6, 2024

adamrodger Oct 6, 2024

adamrodger Oct 6, 2024

		@@ -0,0 +1,17 @@
		using System.Text.Json.Serialization;

		namespace PactNet.Matchers;


		namespace PactNet.Matchers;

		public class ArrayContainsMatcher : IMatcher

feat: add array contains matcher #518

Are you sure you want to change the base?

feat: add array contains matcher #518

Conversation

MaxCampman commented Sep 7, 2024

What?

NOTE:

adamrodger 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

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

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

MaxCampman Sep 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxCampman 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

Choose a reason for hiding this comment

MaxCampman Sep 7, 2024 • edited Loading

Choose a reason for hiding this comment

adamrodger commented Sep 8, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxCampman Sep 7, 2024 •

edited

Loading

MaxCampman Sep 7, 2024 •

edited

Loading