cah4a / dart_nock

HTTP requests mocking library for dart and flutter.
https://pub.dev/packages/nock
MIT License
20 stars 14 forks source link

Post request not correctly mocked #4

Closed rvoortman closed 4 years ago

rvoortman commented 4 years ago

Hey there, great work with Nock. It works pretty awesome so far! I've stumbled on the first problem when I started (m/n)ocking my first POST request. I can't seem to get it to Mock the request correct.

I'm using Nock 1.0.2 using the Flutter test environment.

The minimum code to replicate this issue is as follows:

The class

import 'package:http/http.dart';

class TestClass {
  Future<bool> postTest() async {
    String url = 'https://example.com/awesome';

    Response response = await Client().post(url, headers: {"Content-type": "application/json"}, body: jsonEncode({"some": "data"}));
    if(response.statusCode == 200) {
      return true;
    } else {
      throw("${response.statusCode}: eror request");
    }
  }
}

In the testfile:

  setUpAll(() {
    nock.init();
  });

  setUp(() {
    nock.cleanAll();
  });

   final testClass = TestClass();

    test('should work', () async {
      final interceptor = nock("https://example.com").post("/awesome",
       {"some": "data"}
      )..headers( {"Content-type": "application/json"})
      ..replay(200, {"test": "test"});

      var result = await testClass.postTest();

      expect(result, true);
      expect(interceptor.isDone, true);
    });

Result:

should work:

ERROR: Request was: POST https://example.com/awesome
Pending mocks: (post https://example.com/awesome +b -> 200 {test: test})
package:nock/src/overrides.dart 146:7   MockHttpClientRequest.done
package:nock/src/overrides.dart 181:41  MockHttpClientRequest.close
dart:async                              _completeOnAsyncReturn
package:nock/src/overrides.dart         MockHttpClientRequest.addStream

What I tried:

cah4a commented 4 years ago

Hi @rvoortman! It's good to hear that you are enjoying the package!

The issue is about the headers.

First, header names in dart are converting to the lower-case by dart:http. Thus, nock sees content-type header is requested, but Content-type is expected.

Maybe we should add an optional parameter to the ..headers() method to not bother a developer for such kinds of things.

Another thing that dart:http has special processing of content-type header to meet the http specification. It automatically adds a charset to it.

Now then the real request headers in your case will be as followed:

content-type: application/json; charser=utf-8

If you want to check just mimeType you could use startsWith or contains matcher:

nock("https://example.com").post("/awesome", {"some": "data"})
  ..headers({
      "content-type": contains("application/json"),
  });

Or you could omit the call ..headers() method, so nock will not check headers at all. I've just tried to remove ..headers( {"Content-type": "application/json"}) from your example and it works.

Thanks for your contribution.

rvoortman commented 4 years ago

Thank you for your quick reply. Unfortunatley it didn't quite work. I've tried omiting the ..headers and I still can't get it to work. The following error is thrown:

  Request was: POST https://test.dev/test
  Pending mocks: (post https://test.dev/test -> 200 {test: test})

I removed all the code until the minimum viable code to reproduce is left.

The code to test:

  Map<String, String> headers = {"Content-type": "application/json"};

  Future<bool> testPost() async {
    String url = 'https://test.dev/test';

    Response response = await Client().post(url, headers: headers, body: '{"test": "test"}');

    return response.statusCode == 200;
  }

The test itself:

   final testClass = TestClass();

  test('throws an error', () async {
    final interceptor = nock("https://test.dev").post("/test")..replay(200, {'test': 'test'});

    var result = await testClass.testPost();

    expect(result, true);
    expect(interceptor.isDone, true);
  });

Some other things I tried in the meantime:

It seems the body is not correctly matched if not given; I would assume that not giving the body would not check for that, but it seems it somehow does. Could it because the following line results in a false? https://github.com/cah4a/dart_nock/blob/bb6275fe4eef3d22a4d11436fc436e5011741c50/lib/src/request_matcher.dart#L30

rvoortman commented 4 years ago

Yea I found the problem was in the 'body' part.

When I give the exact body to the post like toe following: .post("/test", '{"test": "test"}') it works correctly. The following also works (since I do not really care about the body in my test):

.post("/test", (body) { return true; } )

which basically always matches the body.

I expected it to be more forgiven and to just match always when the body is not given in the nock post function call. Is that something that is possible to program or that can be documented?

cah4a commented 4 years ago

Yeah, the request body is something that really important in vast majority cases. That forces developers to describe what data should be sent. In my opinion, that kind of checks is very needful.

Also, nock accepts matchers, which is a highly customizable way to describe your request body data. Similar to the second argument of the expect function. Something like:

allOf([
  contains('foo'),
  isNot(startsWith('bar')),
  endsWith('baz')
])

So, if you don't want to do any checks of the body, you could use anything matcher:

.post("/test", anything)

But, there should be a reason to do so.

rvoortman commented 4 years ago

ironically the anything matcher did not really worked in my case, but the workaround is working for us.

Actually, if I'm totally honest with you, I don't think the sended request body is not all that important for testing (at least not what I can think of), since you mostly want to test what happens with a certain response. For example: how does the application handle certain errors or responses?

The only time I want to for the body is when I'm posting specific data that is manipulated in a way, which is not all that often in our case (the api requests we are doing are highly abstracted and easily mocked). So I am not certain if the default behaviour is really what we want (request body matches empty if none is given). Maybe there could be a matcher to test for nothing and just ignore it if no body is given, just like with the headers.

It's an idea. If you do not wish to change the default behaviour, you can close this issue since I can at least continue now :)

BTW: Thank you for replying so fast. It gives confidence. 👍 🎉

cah4a commented 4 years ago

Hey @rvoortman!

Finally got some time to investigate why the body matcher is not working as expected. The new version 1.1.1 I've released recently contains a fix for that.

Sorry for the delay and thanks for your contribution!

rvoortman commented 4 years ago

You're the best! Thank you for your time and effort into this 👍 :)