Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sample of unit testing #70

Closed
stefandevo opened this issue May 6, 2020 · 9 comments
Closed

Add sample of unit testing #70

stefandevo opened this issue May 6, 2020 · 9 comments

Comments

@stefandevo
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Just a clarification.

Describe the solution you'd like
I would like to see how you could unit test https://github.com/jonataslaw/get/blob/master/example/lib/home/repositorys/home_repository.dart the repository, to have another implementation (mock) during testing of the api call.
With get_it for example, you would create another implementation of the abstract class.
Thx.

@jonataslaw
Copy link
Owner

Is your feature request related to a problem? Please describe.
Just a clarification.

Describe the solution you'd like
I would like to see how you could unit test https://github.com/jonataslaw/get/blob/master/example/lib/home/repositorys/home_repository.dart the repository, to have another implementation (mock) during testing of the api call.
With get_it for example, you would create another implementation of the abstract class.
Thx.

I understand that get_it you could create an abstract Service class and implement it, and then test a class that implements it.
However, Get works in another way. You can't, at least until then, register an abstract class, and I don't objectively see why doing that (my opinion may change in the future).

What's wrong with testing your API like that?

void main() {
  test('Test Api', () async {
    Get.put<Repository>(Repository());
    var result = await Get.find<Repository>().init();
    expect('apiSample', result);
  });
}

class Repository {
  Future init() async {
    var apiMock = Future.delayed(Duration(milliseconds: 500), () {
      return 'apiSample';
    });
    return apiMock;
  }
}

I would love to hear your opinion.

@stefandevo
Copy link
Contributor Author

Based upon the sample in this repository, I want to test https://github.com/jonataslaw/get/blob/master/example/lib/home/views/home_view.dart

It is calling fetchDataFromApi in
https://github.com/jonataslaw/get/blob/master/example/lib/home/views/home_view.dart#L30

However, in my test, I want a test where I once want to return 0 items, in another test I want to return 1 item, in another test I want 100 items, in another test I want a network exception.

I therefore need to mock the Api class
https://github.com/jonataslaw/get/blob/master/example/lib/home/repositorys/home_repository.dart

So with the way get_it works, I have one function where I initialize my singletons.
In the app I register the real singletons, in my unit test, I register for each test case a mock class that gives back what I want to test. So I do 4 test, and every time the implemention of the Api class will change.

I don't see it with Get how this can be done because, I understand, the library works different from get_it (most of the time for the better!).

The
https://github.com/jonataslaw/get/blob/master/example/lib/home/controllers/home_controller.dart#L15
takes the concrete implementation of Api, always, and when the Controller is used from within a test, I need 4 other implementations of Api instead.

Sorry if I misunderstood a feature, but for now, I don't see it :-)

@jonataslaw
Copy link
Owner

I mock the http request with the mock of the packages I use. But to make it easier, just give me an example Get_it, which I will give you back how to do it, and if there is no similar API in Get I will include it immediately

@stefandevo
Copy link
Contributor Author

So this is a snippet of using Get_it where I have a service (storage service) which then calls an underlying Api.

test('Given postId 1, should call localStorageService with 1', () async {
      var mockStorageService = MockStorageService();
      locator.registerSingleton<StorageService>(mockStorageService);
      var mockApi = MockApi();
      when(mockApi.likePost(1)).thenAnswer((_) => Future.value(true));
      locator.registerSingleton<Api>(mockApi);
      var postService = PostService();
      await postService.likePost(1);
      verify(mockStorageService.likePost(1));
    });

What is happening here, is that first I register the MockStorageService as an implementation of the abstract class StorageService. Then I register MockApi as an implementation of the abstract class Api (because in the StorageService implementation in the real app, I retrieve the registered singleton for Api.

When mapped to the example in this repository,

I would like to unit test the Controller https://github.com/jonataslaw/get/blob/master/example/lib/home/controllers/home_controller.dart
The Controller is now also getting an instance of Api https://github.com/jonataslaw/get/blob/master/example/lib/home/controllers/home_controller.dart#L15

So I would need to Mock the Controller first, and then also Mock the Api class.

So Controller.fetchDataFromApi() is calling Api.fetchData().

I was thinking of a way to "intercept" the Get.find(); that in the test, you could hook into a callback function, that you can return an instance of something else for each .find(). Of course the same methods, etc should be available in the returned instance of the Mock - for which abstract classes are great ofcourse.

I hope this is more clear? Thanks for your support!

@jonataslaw
Copy link
Owner

Hi, I couldn't make a mock for your example, but I changed the Get Instance Manager API a little, and you can now use abstract classes. This is not a real example, but it also shows how to inject instances into other instances.

abstract class Service {
  Map<String, String> post(int i);
}

class Api implements Service {
  @override
  Map<String, String> post(int i) {
    if (i == 0) {
      return {"Api_status": "200"};
    } else if (i == 1) {
      return {"Api_status": "400"};
    } else {
      return null;
    }
  }
}

class Controller extends GetController {
  Map<String, String> data;
  void getData(int i) {
    data = Get.find<Api>().post(i);
  }
}

void main() {
  test('Test 1', () {
    var api = Get.put<Api>(Api());
    var service = Get.put<Service>(api);
    expect(service.post(0), {"Api_status": "200"});
    expect(service.post(1), {"Api_status": "400"});
    expect(service.post(2), null);
  });

  test('Test 2', () {
    var controller = Get.put<Controller>(Controller());
    controller.getData(0);
    expect(controller.data, {"Api_status": "200"});
    controller.getData(1);
    expect(controller.data, {"Api_status": "400"});
    controller.getData(2);
    expect(controller.data, null);
  });
}

From what I understand from the last part of your comment, would you like a function to return a new instance every time? A kind of factory?

You can use Get.creator() for this.

Get.creator(() => Repl());
Repl a = Get.find();
Repl b = Get.find();
/// print(a==b); (false)

I haven't documented this yet, because it's an experimental API, and I still don't know if it would be a better approach:
Get.creator(() => Repl());
of
Get.creator(Repl());

In addition, when registering an abstract class, you will be able to use numerous concrete classes of it, which it will not return, no longer the same type of Type as before.
If you inject a concrete class that has already been injected into Get inside it, in this case, it will deliver the concrete class already implemented, but for that, you must insert a Get.find<Concrete>() instead of using a class instance inside Get.put<Abstract>(Get.find<Concrete>()); or save Get.find<Concrete>() in a variable and use inside:
var foo = Get.find();
var bar = Get.put(foo);

Remembering that with Get, using Get.put will return the instance that was inserted in Get, as well as Get.find() .

I hope the new API will help you to implement things the way you think. The dependency manager was archaic with the T indexer, modifications were necessary, and well, I don't intend to add more features to this API because I want to keep this lib always light and simplistic, with enough to do anything, but without abstractions that can inflate the lib code. But the current changes were necessary.

@stefandevo
Copy link
Contributor Author

Hi

Point is, that in you sample in the Controller I need to use the abstract class to get the concrete implementation at that moment.

Basically in your Test2 I want to tell the system, not to use Api, but Api2 (with same abstract base class).

So what you do in Test1 is OK, in the test you implement a mock that extends the abstract class, then register the Service and then use the Service -> fine!

But the level above that, is the controller I want to unit test. The controller is using Api directly through data = Get.find<Api>().post(i); and therefore I cannot replace it with something else.

Ideally the line should be data = Get.find<Service>().post(i); and there needs a place where I can tell Get that I want Api as the concrete implementation of Service.

In get_it, you have typically one method you call upon start of the application where you define the concrete implementations of the abstract classes.

I know that this is something you want to avoid (extra boiler plate code), but now it seems impossible to unit test all my layers; the only option I would have is to put a if..then around each find in order to test if we are running in a unit test or not.

@jonataslaw
Copy link
Owner

Hi

Point is, that in you sample in the Controller I need to use the abstract class to get the concrete implementation at that moment.

Basically in your Test2 I want to tell the system, not to use Api, but Api2 (with same abstract base class).

So what you do in Test1 is OK, in the test you implement a mock that extends the abstract class, then register the Service and then use the Service -> fine!

But the level above that, is the controller I want to unit test. The controller is using Api directly through data = Get.find<Api>().post(i); and therefore I cannot replace it with something else.

Ideally the line should be data = Get.find<Service>().post(i); and there needs a place where I can tell Get that I want Api as the concrete implementation of Service.

In get_it, you have typically one method you call upon start of the application where you define the concrete implementations of the abstract classes.

I know that this is something you want to avoid (extra boiler plate code), but now it seems impossible to unit test all my layers; the only option I would have is to put a if..then around each find in order to test if we are running in a unit test or not.

Wow, thanks for that answer. I read 'tests' quickly, and thought you were talking about integration tests. I think I understand your point now, you want to do unit tests, and register a Service class with a real implementation. Well, do you have a problem with Get.put's overrideValue API? Because I think you could use it like this:

import 'package:flutter_test/flutter_test.dart';
import 'package:get/get.dart';

abstract class Service {
  Map<String, String> post(int i);
}

class Api implements Service {
  @override
  Map<String, String> post(int i) {
    if (i == 0) {
      return {"Api_status": "200"};
    } else if (i == 1) {
      return {"Api_status": "400"};
    } else {
      return null;
    }
  }
}

class Controller extends GetController {
  Map<String, String> data;
  void getData(int i) {
    data = Get.find<Service>().post(i);
  }
}

void main() {
  Get.put<Service>(Api(), overrideValue: false);
  Get.put<Controller>(Controller());
  test('Test 1', () {
    var service = Get.find<Service>();
    expect(service.post(0), {"Api_status": "200"});
    expect(service.post(1), {"Api_status": "400"});
    expect(service.post(2), null);
  });

  test('Test 2', () {
   var controller = Get.find<Controller>();
    controller.getData(0);
    expect(controller.data, {"Api_status": "200"});
    controller.getData(1);
    expect(controller.data, {"Api_status": "400"});
    controller.getData(2);
    expect(controller.data, null);
  });
}

I have integration tests that depend on overrideValue = true, as you want to test isolated units, you can use the above approach. I think I forgot to document this API.
Thank you for clarifying exactly what you wanted to do, often due to lack of attention or language barriers (English is not my native language) we ended up getting it wrong.

Is that what you wanted?

@stefandevo
Copy link
Contributor Author

Hi; thx for your reply.

It seems to work when I create a method in my app where a pre-register all concrete classes up-front. So I basically call Get.put<Service>(Api(), overrideValue: false); in the main method of the flutter app. One thing I see here is that at the start of the app, Api() is already instantiated. An option to have a lazySingleton would be great; this way we register the concrete implementation at first, then upon first call, a 'new' instance is created. When you can add that, I think you have both worlds supported (the ones who wants to use abstract classes for unit testing, and the one where no testing is needed).

@jonataslaw
Copy link
Owner

jonataslaw commented May 10, 2020

You can use Get.lazyPutBuilder<Service>(()=>Api()); to this, or Get.lazyPut<Service>(Api());

If you have any problems, do not hesitate to open another issue.
Closing this.

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

No branches or pull requests

2 participants