felangel / mocktail

A mock library for Dart inspired by mockito
https://pub.dev/packages/mocktail
MIT License
617 stars 81 forks source link

"Delegate" function call in mock to other function - never called #201

Closed WieFel closed 5 months ago

WieFel commented 1 year ago

Describe the bug Assume I have a class A which has 2 methods method1 and method2 with the following implementation:

class A {
  void method1() {
    // ... do something
  }

  void method2() {
    method1();
  }
}

In my test, I mock A by extending Mock and use the mocked class AMock in the test:

class AMock extends Mock implements A {}

void main() {
  test('test function delegation', () {
    var a = AMock();

    // with this, I "reproduce" the original behavior of the class A
    when(() => a.method2()).thenReturn(() => a.method1());

    a.method2();
    verify(() => a.method2()).called(1);
    verify(() => a.method1()).called(1); // fails -> method1() is not called
  });
}

When running the test, the verification of the method2 call completes correctly. However, method1 is apparently not called, although it should actually be. Is there anything I am doing wrong here?

To Reproduce Steps to reproduce the behavior:

  1. Copy the complete code into a test file:

    
    class A {
    void method1() {
    // ... do something
    }
    
    void method2() {
    method1();
    }
    }

class AMock extends Mock implements A {}

void main() { test('test function delegation', () { var a = AMock();

// with this, I "reproduce" the original behavior of the class A
when(() => a.method2()).thenReturn(() => a.method1());

a.method2();
verify(() => a.method2()).called(1);
verify(() => a.method1()).called(1); // fails -> method1() is not called

}); }

2. Run the test as unit test.

**Expected behavior**
I would expect that the test calls `method1`, as it is called from `method2`.

**Logs**
Error log of the test:

No matching calls. All calls: [VERIFIED] AMock.method2() (If you called verify(...).called(0);, please instead use verifyNever(...);.)


Paste the output of running `flutter doctor -v` here.

Flutter (Channel beta, 3.12.0, on macOS 13.3.1 22E772610a darwin-x64, locale en-GB) • Flutter version 3.12.0 on channel beta at /Users/felix/fvm/versions/3.12.0 ! Warning: dart on your path resolves to /usr/local/Cellar/dart/2.17.0/libexec/bin/dart, which is not inside your current Flutter SDK checkout at /Users/felix/fvm/versions/3.12.0. Consider adding /Users/felix/fvm/versions/3.12.0/bin to the front of your path. • Upstream repository https://github.com/flutter/flutter.git • Framework revision 8fcb74dbc1 (5 weeks ago), 2023-06-08 13:37:31 -0600 • Engine revision b84ad1ad30 • Dart version 3.1.0 (build 3.1.0-163.1.beta) • DevTools version 2.24.0 • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.3) • Android SDK at /Users/felix/Library/Android/sdk • Platform android-33, build-tools 30.0.3 • ANDROID_HOME = /Users/felix/Library/Android/sdk • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694) • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.3.1) • Xcode at /Applications/Xcode.app/Contents/Developer • Build 14E300c • CocoaPods version 1.11.3

[✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.2) • Android Studio at /Applications/Android Studio.app/Contents • Flutter plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/9212-flutter • Dart plugin can be installed from: 🔨 https://plugins.jetbrains.com/plugin/6351-dart • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)

[✓] VS Code (version 1.79.2) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.66.0

[✓] Connected device (2 available) • macOS (desktop) • macos • darwin-x64 • macOS 13.3.1 22E772610a darwin-x64 • Chrome (web) • chrome • web-javascript • Google Chrome 114.0.5735.198

[✓] Network resources • All expected network resources are available.

! Doctor found issues in 1 category.

richaux commented 1 year ago

hi, I don't think that when(() => a.method2()).thenReturn(() => a.method1()); does reproduce the original behaviour, as method2 doesn't return anything. I think I'd expect a compilation error!

tbh I'm not convinced that mimicking invocations like this adds much value (I appreciate you've crafted a simpler example, so the original purpose might be hidden)... why would anything using AMock care what it gets up to internally? Seems an odd thing to be testing on the face of it. hth (and apols if I've just misunderstood 😎)

WieFel commented 1 year ago

Thanks for the answer. Let me maybe go a bit more into details about the use-case:

I have a class MQTTProvider which handles messaging via MQTT and offers the following methods (amongst others):

  sendValue(String n, dynamic v) {
    // does the actual sending of n and v over MQTT
  }

  sendInt(String n, int v) {
    // does some conversion on v, then sends by calling sendValue()
    sendValue(n, v);
  }

  sendDouble(String n, double v, {int? numPlaces}) {
    // does some conversion on v, then sends by calling sendValue()
    sendValue(n, v);
}

The MQTTProvider is used in many different widgets of my app (a library of ~50 different kinds of components that communicate over MQTT). Some components might send int values and thus use sendInt. Some others might use sendDouble. Others even directly use sendValue.

So, in my tests I am setting up a "general" mock app which covers all these cases. I am mocking the MQTTProvider like so:

class MockMqttProvider extends Mock implements MQTTProvider {}

Now, when writing the tests for every one of these ~50 components, I want to check whether they send the expected values via MQTT, i.e. if sendValue was called with the expected arguments. But as the MQTTProvider is mocked, a call to sendInt of course doesn't delegate to sendValue by default. That's why I am trying to do something like:

when(() => mockMQTTProvider.sendInt(any(), any())).thenReturn(mockMQTTProvider.sendValue);

What I want to avoid in the tests is having to look up for every component which concrete method (sendInt, sendDouble, ...) it uses to write the tests. What matters in the end is just that sendValue gets called.

I hope it is more clear now. On one side, i want to mock the MQTTProvider to be able to check whether a function has ben called with the expected arguments. But on the other side i want to preserve some of its behaviour, which is the correct delegation of function calls...

What would be your suggestion to do in this case?

richaux commented 1 year ago

🙂 thanks, that describes it nicely.

My own personal preference is not to test such delegation in the components! 😐 The reasoning is that if a particular component has a contract to send stuff via sendInt, then it doesn't care that sendValue gets called, it's only responsible for calling sendInt.

What I want to avoid in the tests is having to look up for every component which concrete method (sendInt, sendDouble, ...) it uses to write the tests.

I think this is ok -- the components just use a vanilla MockMQTTProvider, no customisation required.

What matters in the end is just that sendValue gets called.

The responsibility for such correct delegation resides just in the MQTTProvider's own tests.

I appreciate this may not be the mocktail answer that you came here for! (I don't have any connection with the team, was just passing to thank them for their v1.0.0 efforts!). I hope the snippet below conveys what I'm on about.

It feels like you'd end up with reduced setup in your component tests yet added comfort around your Provider delegation; apols if they're no use at all. 🙂


Possible MQTTProvider implementations: one sets a sendType field you can interrogate; the other doesn't need the field but alters your API to return the type. [Not sure I like the String type but hope it makes sense.]

    String get sendType => _sendType;

  void sendValue(String n, dynamic v) {
    sendValue(n,v,"dyn");
  }

  void sendValue(String n, dynamic v, String sendType) {
    _sendType = sendType;
    // does the actual sending of n and v over MQTT
  }

  void sendInt(String n, int v) {
    // does some conversion on v, then sends by calling sendValue()
    sendValue(n, v, "int");
  }

  sendDouble(String n, double v, {int? numPlaces}) {
    // does some conversion on v, then sends by calling sendValue()
    sendValue(n, v, "dbl");
}

Alternate without field but setting return type.

  String sendValue(String n, dynamic v) {
    // does the actual sending of n and v over MQTT
    return "dyn";
  }

  String sendInt(String n, int v) {
    sendValue(n, v);
    return "int";
  }

  String sendDouble(String n, double v, {int? numPlaces}) {
    sendValue(n, v);
    return "dbl";
}

And tested like:

    final sut = MQTTProvider();
    test('correct route for int', () {
      sut.sendInt("xxx", 1);
      expect(sut.sendType, "int");
    });

or

    final sut = MQTTProvider();
    test('correct route for int', () {
      final result = sut.sendInt("xxx", 1);
      expect(result, "int");
    });
felangel commented 5 months ago

Closing for now since it's been a while but if this is still an issue feel free to leave a comment and I'm happy to continue the conversation 👍