chore: export some things for use by consumers, docs fixes, updates to the match not found message #26

btrautmann · 2023-04-13T02:06:24Z

📰 Summary of changes

What is the new functionality added in this PR?

Ran into a few hiccups when integrating with 0.3.0 that I'm addressing here, namey:

exporting CharlatanHttpRequest and CharlatanRequestMatcher which are helpful for defining custom CharlatanRequestMatchers like the following:

CharlatanRequestMatcher isGraphQLQuery() {
  return requestMatchesAll([
    requestMatchesHttpMethod('POST'),
    requestMatchesPathOrTemplate('/api/graphql'),
  ]);
}

A minor docs typo fix
⚠️ Adding RequestOptions#data to the message that gets printed out when a match cannot be found. This felt helpful because for GraphQL cases, the method and path are always the same, and the query itself (found in data) is what helps identify the request for which there was no response definition. HOWEVER, at the same time, GraphQL queries can be pretty chunky, so when printing this out, there's a lot to parse. Should we truncate the data (often the first little bit is enough to see what was being requested) or do something else entirely? UPDATE: Pulled this change out, see discussion on PR

🧪 Testing done

What testing was added to cover the functionality added in this PR

I've updated the test for the one functional change, but that's the part of this PR that's most up for discussion!

…dditional data to the message printed when a match cannot be found

codecov · 2023-04-13T02:09:41Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.56 🎉

Comparison is base (4a8b5bc) 95.34% compared to head (eefe412) 95.91%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #26      +/-   ##
==========================================
+ Coverage   95.34%   95.91%   +0.56%     
==========================================
  Files           4        4              
  Lines          86       98      +12     
==========================================
+ Hits           82       94      +12     
  Misses          4        4

Impacted Files	Coverage Δ
lib/charlatan.dart	`100.00% <ø> (ø)`
lib/src/charlatan.dart	`100.00% <100.00%> (ø)`
lib/src/charlatan_http_client_adapter.dart	`87.87% <100.00%> (-1.32%)`	⬇️
lib/src/charlatan_response_definition.dart	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

samandmoore

Curious if you like my suggestion!

samandmoore · 2023-04-13T12:59:08Z

lib/src/charlatan_http_client_adapter.dart

@@ -44,7 +44,7 @@ class CharlatanHttpClientAdapter implements HttpClientAdapter {

 Unable to find matching fake http response definition for:

-${method.toUpperCase()} $path
+${method.toUpperCase()} $path ${options.data}


I feel ya on the struggle of making this readable for gql or other nuanced requests.

Two thoughts:

We could probably do something to include the query string here in addition to just the path. That won't help for gql unless we make some tweaks to clients to include the operation name in the query string. Which would be really useful

We could add a hook on Charlatan to allow you to customize how the request prints out for this debugging use case with a solid default. Then consumers can do whatever makes sense to them to make the failed match on the request easier to interpret. Like in the case of gql maybe you can parse the operation name from the body instead of using the whole body. Not sure of a great name for this. Maybe requestDebugFormatter? Naming this is hard haha

What do you think?

We could add a hook on Charlatan to allow you to customize how the request prints out for this debugging use case with a solid default.

I like this idea as it was something I had been thinking about for the actual "matching" logic. I was struggling to suss out what Charlatan was doing under the hood and breakpoints were helpful but didn't fully solve my struggles so I pulled down Charlatan locally and added some logs like:

Matching $requestInfo against $matcherDescription; did not match Matching $requestInfo against $otherMatcherDescription; did match ...

That along with the actual "couldn't find matches, here's what's configured:" message could be value adds, but I'd like to iterate on that in a follow-up PR because the real need from this PR is the exports of the typedefs. So I'll back these changes out and maybe open an issue for this stuff.

samandmoore

domainlgtm

samandmoore

platformlgtm

Dismissed because the approval was invalidated by another commit

samandmoore

domainlgtm

samandmoore

platformlgtm

…refactor * origin/main: chore: export some things for use by consumers, docs fixes, updates to the match not found message (#26)

btrautmann added 2 commits April 12, 2023 21:48

chore: export some things for use by consumers, docs fixes, and add a…

63a566f

…dditional data to the message printed when a match cannot be found

update test

efbe5ae

btrautmann requested a review from a team as a code owner April 13, 2023 02:06

samandmoore reviewed Apr 13, 2023

View reviewed changes

btrautmann mentioned this pull request Apr 13, 2023

Support additional debugging verbosity #27

Open

back out changes to error logs

8c8eb89

samandmoore previously approved these changes Apr 13, 2023

View reviewed changes

bump version, adjust CHANGELOG

eefe412

btrautmann dismissed samandmoore’s stale review via eefe412 April 13, 2023 14:08

samandmoore approved these changes Apr 13, 2023

View reviewed changes

btrautmann merged commit 0fad2e8 into main Apr 13, 2023

btrautmann deleted the bt/minor-updates branch April 13, 2023 14:18

samandmoore added a commit that referenced this pull request Apr 13, 2023

Merge remote-tracking branch 'origin/main' into sam-response-builder-…

3172ce4

…refactor * origin/main: chore: export some things for use by consumers, docs fixes, updates to the match not found message (#26)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: export some things for use by consumers, docs fixes, updates to the match not found message #26

chore: export some things for use by consumers, docs fixes, updates to the match not found message #26

btrautmann commented Apr 13, 2023 •

edited

Loading

codecov bot commented Apr 13, 2023 •

edited

Loading

samandmoore left a comment

samandmoore Apr 13, 2023

btrautmann Apr 13, 2023

samandmoore left a comment

samandmoore left a comment

samandmoore left a comment

samandmoore left a comment

chore: export some things for use by consumers, docs fixes, updates to the match not found message #26

chore: export some things for use by consumers, docs fixes, updates to the match not found message #26

Conversation

btrautmann commented Apr 13, 2023 • edited Loading

📰 Summary of changes

🧪 Testing done

codecov bot commented Apr 13, 2023 • edited Loading

Codecov Report

samandmoore left a comment

Choose a reason for hiding this comment

samandmoore Apr 13, 2023

Choose a reason for hiding this comment

btrautmann Apr 13, 2023

Choose a reason for hiding this comment

samandmoore left a comment

Choose a reason for hiding this comment

samandmoore left a comment

Choose a reason for hiding this comment

samandmoore left a comment

Choose a reason for hiding this comment

samandmoore left a comment

Choose a reason for hiding this comment

btrautmann commented Apr 13, 2023 •

edited

Loading

codecov bot commented Apr 13, 2023 •

edited

Loading