Skip to content

Conversation

JeffryGonzalez
Copy link
Contributor

Ok - not sure this is the right approach, but I sort of messed up the StatusCodeShouldBeSuccess() Assertion last year.

I actually noticed this when recording a video with @jeremydmiller. ;(

I feel like I made too big of a change here, and don't mind rejection or feedback.

The issue is the StatusCodeShouldBeSuccess() collided with the default (StatusCodeShouldBeOk).

Since StatusCodes are pretty binary - you wouldn't have multiple checks for different status codes in the same scenario, the Scenario.cs has a IgnoreStatusCode() you can use. so in order for the StatusCodeShouldBeSuccess() to work, you'd have to do something like:

 var response = await M.Host.Scenario(api =>
        {
             api.Post.Json(request).ToUrl("/work-to-be-done");
             api.IgnoreStatusCode(); // <-- add this.
             api.StatusCodeShouldBeSuccess();    
        });

That seems weird to me.

What I propose, meekly, with this PR is (egads) a "Marker Interface" for any Assertion that deals with Status Codes. (IStatusCodeAssertion).

In the Scenario.cs's RunAssertions, you set _ignoreStatusCode to true (if it isn't already) if there are any _assertions that implement that interface.

Now, the above can be written as:

 var response = await M.Host.Scenario(api =>
        {
             api.Post.Json(request).ToUrl("/work-to-be-done");
             // api.IgnoreStatusCode(); // <-- remove this
             api.StatusCodeShouldBeSuccess();    
        });

I considered throwing something if there are more than one assertion that is IStatusCodeAssertion, but that feels too heavy handed.

If this is accepted (and I can't help but think there might be a better way, or someone will see a problem with this), I'd be willing to update the documentation at https://jasperfx.github.io/alba/scenarios/assertions.html.

Thanks, and sorry?

@JeffryGonzalez
Copy link
Contributor Author

I'd say for now forget about this in light of the comment @jeremydmiller made about the changes to Wolverine default status codes (204, etc)

It seems changing Alba's default to "if you don't specify a status code it will check for 200" to "...it will check for 200-299" would be easier and not require what is in this PR.

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.

1 participant