Expose ReadBody on ScenarioAssertionException #133
Conversation
| /// <param name="context">The context of the response</param> | ||
| /// <returns>A string with the content of the body</returns> | ||
| [MemberNotNull(nameof(Body))] | ||
| public string ReadBody(HttpContext context) |
There was a problem hiding this comment.
It's kind of annoying that I have to call ReadBody to assign the Body property and its not very discoverable to end users that this is required and that there is a relationship between the two.
The ReadBody method itself is useful since it will rewind the stream back to the beginning, which could affect other assertions helpers. Perhaps ReadBody should actually be an extension method on HttpContext instead and have a similar to the helper methods in System.Net.Http.Json, e.g. ReadResponseBodyAsText.
There was a problem hiding this comment.
The intention of ScenarioAssertionException's Body property is for it to only be set for assertions that involve the body, as it showing up in error output for non-body assertions isn't particularly useful. The function call hits two birds with one stone, if the user accesses the body in the assertion it also sets it to appear in the error output.
I think we could improve discoverability/ergonomics by wrapping the HttpContext & exception inside of a AssertionContext, but that'd need to wait until the next major release (likely around .NET 8). Near-term we can look into exposing some of the internal helpers as extensions on the HttpContext to at least make assertion writing a tad easier.
|
@Hawxy @egil This would have to wait for V8 like you already said, but I think I'd vote to eliminate the I think I also like the idea of any assertions in Scenario that assert on the contents of the body that they do their thing, but then we rewind the response stream so as not to trip up users doing secondary reads of that content. That's all from thinking about this for just a minute or two though. |
Closes #132