False positive on _httpTest.ShouldHaveCalled()

[LUS1N]

I don't know if it's intended behavior, but it got me sidetracked a bit and I had to make special checking that would be avoided if this worked as expected.

I ran into the issue that when confirming that a url was called, the HttpTest library doesn't compare if the urls are equal but only if the test url is in the called url. Even though there are no wildcards.

Calling /something and /something/1/else/3 are very different things, but the test sees them as equivalent.

To recreated - this test will pass:

using System.Net.Http;
using Flurl;
using Flurl.Http;
using Flurl.Http.Testing;
using Xunit;

namespace FlurlTest
{
    public class MyWebClient
    {
        public string DoThing()
        {
            var url = "https://example.com"
                .AppendPathSegment("somePath/asd/asd");

            var response = url.PostJsonAsync(new { name = "TestBoi" }).Result;

            return response.Content.ReadAsStringAsync().Result;
        }
    }

    public class UnitTest1
    {
        private readonly HttpTest _httpTest;

        public UnitTest1()
        {
            _httpTest = new HttpTest();
        }

        [Fact]
        public void Test1()
        {
            _httpTest.RespondWithJson(new
            {
                ok = true,
                user = new
                {
                    id = "123"
                }
            });

            var client = new MyWebClient();
            var result = client.DoThing();

            _httpTest.ShouldHaveCalled("https://example.com/somePath")
                .WithVerb(HttpMethod.Post)
                .Times(1);
        }
    }
}

.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.0</TargetFramework>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Flurl.Http" Version="2.3.1" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.7.0" />
    <PackageReference Include="xunit" Version="2.3.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.3.1" />
    <DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1" />
  </ItemGroup>

</Project>

[tmenier]

This is a tough call. I'm inclined to agree with you about how it probably should work, but this would be a breaking change that could affect lots of people. I can also kind of see a counter-argument for something like query strings. If your code calls https://api.com/endpoint?x-y, then should this pass?

test.ShouldHaveCalled("https://api.com/endpoint");

Some might say yes. If I had it to do all over again, I'd probably say no, just put a * at the end of it if it might have a query string. But as it is, I don't think I want to take the breaking change at least until the next major version.

I'll keep this open. Maybe there should just be a new method like ShouldHaveCalledExact (and better documentation of ShouldHaveCalled). I'll give it some thought. Thanks for bringing it up.

[bchavez]

Here's how I resolved the issue (with Fluent Assertions):

public static HttpCallAssertion ShouldHaveExactCall(this HttpTest test, string exactUrl)
{
   test.CallLog.First().FlurlRequest.Url.ToString().Should().Be(exactUrl);
   return new HttpCallAssertion(test.CallLog);
}

The reason I needed to match exact URLs was because I had two distinct endpoints /users and /user and was bewildered why following unit test passed:

using( var test = new HttpTest() ){

   var response =  await "https://test.com/v2/users"
                     .PutJsonAsync(new {foo="hi"});

   test.ShouldHaveCalled("https://test.com/v2/user");
   "test should not pass".Dump();
}

Infact, test.ShouldHaveCalled("https") would work too. The behavior of ShouldHaveCalled(...) really has a semantic nature of ShouldHaveCalledUrlStartingWith(...).

Good thing I caught this, because some API calls to /users and /user would have slipped through the cracks.

[tmenier]

This one's finally ready! As a side-note, the same simple wildcard matching is used for other things like asserting query parameters, headers, and response bodies. This will affect all of them, but I'm a little bit lenient in the case of WithConentType. If the full Content-Type header is application/json; charset=utf-8 and WithContentType("application/json") is called, that assertion will pass.

[tmenier]

Now available to test: https://www.nuget.org/packages/Flurl.Http/3.0.0-pre2

[bchavez]

Thanks a bunch Todd! :) I think this is going to really help!

[Kesmy]

Just need to leave a note here that this change destroyed all of the testing I was previously doing with Flurl, and my tests now have to be extremely tightly coupled to implementations because of it -- instead of setting up a service with a URL base and then checking to ensure that it was used, testing now requires that you know the exact URL eventually resolved by that service implementation internally.

A ShouldHaveCalled("http://foo.com/*") also now literally expects a call of "http://foo.com/*", rather than converting it to a regex wildcard (in 3.2.4), or else this would not be an issue.

[tmenier]

@Kesmy

A ShouldHaveCalled("http://foo.com/") also now literally expects a call of "http://foo.com/"

Not true and supported by tests. Are you sure the trailing slash isn't the problem?

[Kesmy]

It would be strange for that to be a problem. That said, I attempted a slash, no slash, with wildcard, and without, none of which worked against a call to http://foo.com/whatever/something/athirdthing

[tmenier]

@Kesmy Sorry for the delay on this, been very short on time in the last couple months.

I think this issue was left open by mistake but before I close it I want to understand if there's still an issue. If you're suggesting that ShouldHaveCalled("http://foo.com/*") is NOT matching a call to a URL starting with exactly "http://foo.com/", then I'll need to see a complete example of that. There are numerous tests demonstrating this works (here for example) and I'll need to get a better understanding of what's happening in your case.