mockito icon indicating copy to clipboard operation
mockito copied to clipboard

[proposal] Alternatives for code generation with NNBD

Open pedromassango opened this issue 5 years ago • 13 comments

Based on NULL_SAFETY_README.md looks like Mockito will be using code gen solution due to limitations introduced by NNBD explained in the same file. I'm creating this issue to discuss some alternative solutions since code_gen is clearly a big step back for Mockito.

Proposal

The file metioned above says that the problem is that Mockito need a default value that can be used for every parameter and with NNBD we can't use null because some parameters might be non-nullable. I think one option would be to make everything nullable and leave the developer make sure the API is used properly.

Note: share your alternative solutions in the comments.

pedromassango avatar Oct 11 '20 08:10 pedromassango

I think one option would be to make everything nullable and leave the developer make sure the API is used properly.

The developer is free to do this. Code generation is not required when using Mockito under null safety. If the developer wishes to mock or verify methods with non-nullable parameters or return types, then code generation is one option. Manual mock implementation is another.

srawlins avatar Oct 12 '20 15:10 srawlins

The problem is that mocking manually requires a lot of boilerplate code.

Argument matchers

Mockito's helpful argument matchers like any, argThat, captureAny, etc. all return null. In the code above, null is being passed to start. null is used because, before null safety, it is the only value that is legally assignable to any type. Under null safety, however, it is illegal to pass null where a non-nullable int is expected.

What if we create alternative typed getters/methods that does not allow nullable values? We can even have it as extensions of the existing matchers.

Here in the Sample 1 bellow I've created a getter to allow me to pass in any argument of type String, we can build one for each Dart's data type:

  • String get anyString => any as String;
    • We can use this in non-nullable arguments of type String.
Sample 1
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';

enum LoginResult { success, invalidEmail, invalidPwd }

class MockAPIService extends Mock implements APIService {}

class APIService {
  LoginResult login(String email, String pwd) {
    return email.isEmpty
        ? LoginResult.invalidEmail
        : pwd.isEmpty ? LoginResult.invalidPwd
        : LoginResult.success;
  }
}

void main() {
  test('RegEx matcher', () {
    final data = [
      '[email protected]',
      '[email protected]',
      '[email protected]'
    ];
    final pattern = r'(\w+)(?:@)(\w+)';

    data.forEach((element) {
      final r = RegExp(pattern);
      r.allMatches(element).forEach((resultItem) {
        final username = resultItem.group(0);
        final emailDomain = resultItem.group(1);
        //final emailDomain2 = resultItem.group(2);
        print('$username : $emailDomain Cc: ${resultItem.groupCount}');
      });
    });
  });

  test('NNBD', () {
    var server = MockAPIService();
    when(server.login(anyString, '')).thenReturn(LoginResult.invalidPwd);

    final result = server.login('[email protected]', '');
    expect(result, LoginResult.invalidPwd);
  });
}

String get anyString => any as String;

Sample 2

Here I just did the same one getter for integers and one function for any type T:

  • int get anyInt => any as int;
  • T anything<T>() => any as T;
    • We can pass this as a non-nullable argument for parameters of type T.
Sample 2
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';

class MockHttpServer extends Mock implements HttpServer {}
class HttpServer {
  Uri start(int port)  {
    return Uri(port: port);
  }

  Uri doNothing(Uri uri)  {
    return uri;
  }
}


void main() {
  test('RegEx matcher', () {
    final data = [
      '[email protected]',
      '[email protected]',
      '[email protected]'
    ];
    final pattern = r'(\w+)(?:@)(\w+)';

    data.forEach((element) {
      final r = RegExp(pattern);
      r.allMatches(element).forEach((resultItem) {
        final username = resultItem.group(0);
        final emailDomain = resultItem.group(1);
        //final emailDomain2 = resultItem.group(2);
        print('$username : $emailDomain Cc: ${resultItem.groupCount}');
      });
    });
  });

  test('NNBD', () {
    var server = MockHttpServer();
    var uri = Uri.parse('http://localhost:8080');
    // server.start
    when(server.start(anyInt)).thenReturn(uri);
    final result1 = server.start(0000);
    expect(result1.port, 8080);
    // server.doNothing
    when(server.doNothing(anything())).thenReturn(Uri(port: 9999));
    final result2 = server.doNothing(Uri());
    expect(result2.port, 9999);
  });
}

int get anyInt => any as int;
T anything<T>() => any as T;

@srawlins is there any problem with the suggesttions above?

pedromassango avatar Oct 12 '20 18:10 pedromassango

These examples result in runtime errors:

String get anyString => any as String;

The cast (as) will fail, as any returns null (not a non-nullable String).

srawlins avatar Oct 12 '20 19:10 srawlins

These examples result in runtime errors:

String get anyString => any as String;

The cast (as) will fail, as any returns null (not a non-nullable String).

Have you tried it? I have NNBD enabled and the tests pass!

Edit: Without this the only problem I faced was from the analyzer saying "can't pass Null into String".

pedromassango avatar Oct 12 '20 19:10 pedromassango

If I write this file:

void main() {
  String s = anyString;
}

String get anyString => null as String;

into a package with this SDK constraint:

environment:
  sdk: '>=2.10.0 <3.0.0'

and run the script with the non-nullable experiment:

$ dart --enable-experiment=non-nullable a.dart

then I get the following error:

Unhandled exception:
type 'Null' is not a subtype of type 'String' in type cast
#0      anyString (file:///Users/srawlins/code/dart-markdown/a.dart:5:30)
#1      main (file:///Users/srawlins/code/dart-markdown/a.dart:2:14)
#2      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:301:19)
#3      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)

srawlins avatar Oct 12 '20 20:10 srawlins

Pretty much same thing on NS Dartpad too...

Screenshot 2020-10-13 at 08 27 52

maks avatar Oct 12 '20 21:10 maks

Hm, looks live I've just enabled NNBD for the analyzer and not actually running the code with it. Just tried on Dartpad and clearly don’t work.

pedromassango avatar Oct 13 '20 14:10 pedromassango

This is maybe a silly question, but since Never will be the new bottom type, why cannot your use Never instead of Null in these cases?

letsar avatar Oct 15 '20 05:10 letsar

One reason is that you cannot instantiate Never. Mockito would still need a placeholder object, a replacement for null which represents Never.

I think there are other reasons, involving how Never works. Like if Mock's noSuchMethod returned Never...

srawlins avatar Oct 15 '20 13:10 srawlins

@srawlins It is possible to overcome this by using reflections? Just asking since I don't have knowledge in this field.

pedromassango avatar Oct 21 '20 18:10 pedromassango

Hmm good question. Dart's runtime-reflection library is called dart:mirrors. The question is how to preserve the current API (when(obj.m(any))) and support methods with non-nullable parameters (and return types).

You could conceive of any API where any is a generic function like T any<T>() {...}, so you would write: when(obj.m(any<Foo>())) and the any function would maybe create a ClassMirror of T, and create a new instance of it, and return that. Perhaps similarly for the return type. I'm not certain you actually can do this, but it sounds like it might lead to something.

I see two problems with reflection though:

  • This requires an API change, converting all of the current argument matchers (any, anyNamed, argThat, etc) to something new. We're not currently looking to pay the price of changing the API.
  • Runtime reflection with dart:mirrors is very slow and creates huge binaries. As in, unacceptably slow and huge.

srawlins avatar Oct 21 '20 19:10 srawlins

for anyone looking into reflection, just a heads up, so you don't waste your time: https://github.com/dart-lang/sdk/issues/44310#issuecomment-733620890

knaeckeKami avatar Nov 25 '20 11:11 knaeckeKami

fwiw I’ve been messing around with an alternative mocking API to address some of these issues at https://pub.dev/packages/mocktail.

felangel avatar Jan 05 '21 02:01 felangel