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

Implement assert_matches with expanded expressions support in match assertions #5024

Merged
merged 13 commits into from
Jan 31, 2025

Conversation

zoldar
Copy link
Contributor

@zoldar zoldar commented Jan 28, 2025

Changes

This is a proposal for implementing a more expressive match assertion mechanism. It was prompted by the original idea by @macobo in #5019.

The idea here is that the pin (^) operator does not only rebind existing binding in the scope but also allows embedding basically any other expression to match against the given part of the pattern. Normal pattern matching is also supported and both can be mixed. The only caveat so far is that when normal patterns fail, only they are listed in the error even if there are potentially failing expressions. However, once the normal pattern is fixed, they surface.

Currently, the following expressions can be pinned:

  • any(:atom)
  • any(:string)
  • any(:binary)
  • any(:integer)
  • any(:float)
  • any(:boolean)
  • any(:map)
  • any(:list)
  • all above variants of any with a one argument predicate function accepting value and returning a boolean, like: any(:integer, & &1 > 20)
  • a special case of any(:string, ~r/regex pattern/) checking that value is a string and matches a pattern
  • shorthand version of the above, ~r/regex pattern
  • any artibrary one argument function returning a boolean, like &is_float/1 or &(&1 < 40 or &1 > 300)
  • exact(expression) where expression is compared using equality, so that can enforce full equality inside a pattern, like: exact(%{foo: 2}) which will fail if the value is something like %{foo: 2, other: "something}
  • any other arbitrary expression which is compared the way as if it was wrapped with exact - this allows "interpolating" values from schemas and maps without rebinding like user.id (instead of having to rebind to user_id first)
    n = %{z: 2}

    assert_matches %{
                     a: ^any(:integer, &(&1 > 2)),
                     b: ^any(:string, ~r/baz/),
                     d: [_ | _],
                     e: ^~r/invalid/,
                     f: ^n.z,
                     g: ^(&is_float/1),
                     h: ^exact(%{foo: :bar})
                   } = %{
                     a: 1,
                     b: "twofer",
                     c: :other,
                     d: [1, 2, 3],
                     e: "another string",
                     f: 1,
                     g: 4.2,
                     h: %{foo: :bar, other: "stuff"}
                   }
  end

image

Although the macro seems to be relatively feature complete, more testing against some real world scenarios must be done before merging.

@zoldar zoldar requested a review from a team January 28, 2025 10:13
@zoldar zoldar force-pushed the assert-experiment branch from 43dd4e4 to 9608a34 Compare January 28, 2025 10:54
@zoldar
Copy link
Contributor Author

zoldar commented Jan 28, 2025

@macobo suggested it would be good to have a way to enforce exact set of keys within a potentially nested map pattern.

I think it's feasible with an extra pass in the macro, and the syntax for such pattern would look roughly like:

      assert_matches  %{
        "results" => [%{"dimensions" => ["some phrase"], "metrics" => [1, 1, 100.0]}],
        "meta" => ^strict_map(%{
          "imports_included" => false,
          "imports_skip_reason" => "unsupported_query",
          "imports_warning" =>
            ~r/Imported stats are not included in the results because query parameters are not supported/
        }),
        "query" =>
          ^exactly(response_query(site, %{
            "metrics" => ["visitors", "events", "conversion_rate"],
            "dimensions" => ["event:props:search_query"],
            "filters" => [
              ["is", "event:goal", ["WP Search Queries"]]
            ],
            "include" => %{"imports" => true}
          }))
      } = json_response(conn, 200)

the idea here is that ^strict_map pins are matched first (with a check boiling down to map_size(value) == count_of_keys_in_pattern). Then the ^strict_map is replaced in the pattern with its contents in the AST and passed on to the existing check. I'll have a stab at it some time later on.

@zoldar
Copy link
Contributor Author

zoldar commented Jan 29, 2025

I have added some basic support for enforcing keys with ^strict_map operator in ef3c33d (along with a fix for an unrelated case of properly handling normal pins and bindings).

It's now possible to do something like:

        assert_matches ^strict_map(%{
                         "id" => ^any(:integer),
                         "name" => "Some segment",
                         "type" => ^"#{unquote(type)}",
                         "segment_data" => %{"filters" => [["is", "visit:entry_page", ["/blog"]]]},
                         "owner_id" => ^user.id,
                         "inserted_at" => ^any(:string),
                         "updated_at" => ^any(:string)
                       }) = response

The error is not ideal yet though ("updated_at" was commented out for this one):

image

More importantly though, the current implementation does not allow nesting ^strict_map's, so it's not possible to do patterns like (the inner one using ^exactly would work though):

        assert_matches ^strict_map(%{
                         "id" => ^any(:integer),
                         "name" => "Some segment",
                         "type" => ^"#{unquote(type)}",
                         "segment_data" => ^strict_map(%{"filters" => [["is", "visit:entry_page", ["/blog"]]]}),
                         "owner_id" => ^user.id,
                         "inserted_at" => ^any(:string),
                         "updated_at" => ^any(:string)
                       }) = response

This can be made to work but I'll shelve it for now and come back to it later. It requires a separate match operation per each strict_map binding to work properly. Also, the whole macro needs a refactor because it's getting unwieldy. 🙈

@zoldar zoldar force-pushed the assert-experiment branch from ef3c33d to 7788262 Compare January 29, 2025 19:47
@zoldar zoldar force-pushed the assert-experiment branch from 7788262 to 45dc56b Compare January 29, 2025 19:47
@zoldar
Copy link
Contributor Author

zoldar commented Jan 29, 2025

Alright, I think I got it functionally at least, the code's still not pretty:

        assert_matches ^strict_map(%{
                         "id" => ^any(:integer),
                         "name" => "Some segment",
                         "type" => ^"#{unquote(type)}",
                         "segment_data" =>
                           ^strict_map(%{"filters" => [["is", "visit:entry_page", ["/blog"]]]}),
                         "owner_id" => ^user.id,
                         "inserted_at" => ^any(:string),
                         # "updated_at" => ^any(:string)
                       }) = response

The report:

image

@zoldar
Copy link
Contributor Author

zoldar commented Jan 29, 2025

Also, added some additional built-in checks for common data types and date/time string formats, because why not:

        assert_matches ^strict_map(%{
                         "id" => ^any(:pos_integer),
                         "name" => "Some segment",
                         "type" => ^"#{unquote(type)}",
                         "segment_data" =>
                           ^strict_map(%{"filters" => [["is", "visit:entry_page", ["/blog"]]]}),
                         "owner_id" => ^user.id,
                         "inserted_at" => ^any(:iso8601_naive_datetime),
                         "updated_at" => ^any(:iso8601_naive_datetime)
                       }) = response

@zoldar zoldar marked this pull request as ready for review January 29, 2025 21:12
@zoldar zoldar requested a review from a team January 29, 2025 21:12
@zoldar
Copy link
Contributor Author

zoldar commented Jan 30, 2025

@apata @macobo It's ready if you'd like to give it a spin.

Copy link
Contributor

@macobo macobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great and I appreciate the extra functionality! 💪

@zoldar zoldar added this pull request to the merge queue Jan 31, 2025
Merged via the queue into master with commit 62c6cb5 Jan 31, 2025
8 checks passed
@zoldar zoldar deleted the assert-experiment branch January 31, 2025 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants