google / protobuf.dart

Runtime library for Dart protobufs
https://pub.dev/packages/protobuf
BSD 3-Clause "New" or "Revised" License
527 stars 183 forks source link

Generated client calls should use more general Future over ResponseFuture #162

Open fuzzybinary opened 5 years ago

fuzzybinary commented 5 years ago

Looking into https://github.com/grpc/grpc-dart/issues/114 it looks like clients currently generate calls that look like the following:

ResponseFuture<HelloReply> sayHello(HelloRequest request,
      {CallOptions options}) {
    final call = $createCall(_$sayHello, new Stream.fromIterable([request]),
        options: options);
    return new ResponseFuture(call);
}

ResponseFuture is an implementation detail that only has private methods. Using this form makes writing mocks for generated clients harder. Is there any reason we couldn't replace ResponceFuture w/ Future in the generated signature?

sigurdm commented 5 years ago

While I agree that the design is not optimal I don't think it's true that ResponseFuture has no non-private methods. It mixes in _ResponseMixin so it can be cancelled and its headers and trailers can be inspected

fuzzybinary commented 5 years ago

Ah, sorry, this stemmed from my misunderstanding of how public members on private mixins work.

What would be a better method of allowing mocking of ResponseFuture returns from gRPC mocks? Would it be worth supplying a ResponseFuture.value constructor similar to Future.value?

facundomedica commented 5 years ago

I'm having problems to mock my gRPC methods too, is there any work arounds?

MarcinusX commented 5 years ago

I've managed to create a workaround for that.
The only tricky part is to create a Mock for ResponseFuture:

class MockResponseFuture<T> extends Mock implements ResponseFuture<T> {
  final T value;

  MockResponseFuture(this.value);

  Future<S> then<S>(FutureOr<S> onValue(T value), {Function onError}) =>
      Future.value(value).then(onValue, onError: onError);
}

Having that, we can do tests like that:

test('Test grpc Client', () async {
  //init grpc client
  client = MockClient();
  //mock method
  when(client.getHealth(any)).thenAnswer(
    (_) => MockResponseFuture<Health>(
        Health()..status = Health_HealthStatus.HEALTH_STATUS_SERVING),
  );

  //call client
  var health = await client.getHealth(Empty());

  //it passes!!!
  expect(health.status, Health_HealthStatus.HEALTH_STATUS_SERVING);
});
PhloxDev commented 4 years ago

@MarcinusX how to generate Grpc error in MockResponseFuture?

MarcinusX commented 4 years ago

@PhloxDev This is what I am using now:

class MockResponseFuture<T> extends Mock implements ResponseFuture<T> {
  final Future<T> future;

  MockResponseFuture.value(T value) : future = Future.value(value);

  MockResponseFuture.error(Object error) : future = Future.error(error);

  MockResponseFuture.future(this.future);

  Future<S> then<S>(FutureOr<S> onValue(T value), {Function onError}) =>
      future.then(onValue, onError: onError);
}
track02 commented 4 years ago

Many thanks for this, really helpful- is it possible to do something similar with ResponseStream in order to simulate a stream of values coming from my client?

Wasn't able to figure it out from first glance but will keep trying, any assistance would be greatly appreciated

songfei commented 4 years ago

I am using this:

class FakeClientCall<Q, R> extends ClientCall<Q, R> {
  FakeClientCall(
    this.result,
  ) : super(null, null, CallOptions()) {
    result.then((value) {
      _responses.add(value);
      _responses.close();
    }).catchError((error) {
      _responses.addError(error);
      _responses.close();
    });
  }

  final Future<R> result;
  final StreamController<R> _responses = StreamController<R>();

  @override
  Stream<R> get response => _responses.stream;
}

useage:

class FakeHelloServiceClient implements HelloServiceClient {
  @override
  ResponseFuture<HelloResponse> hello(HelloRequest request, {CallOptions options}) {
      return ResponseFuture(FakeClientCall<dynamic, HelloResponse>(_hello(request)));
  }

  Future<HelloResponse> _hello(HelloRequest request) async {
      ...
  }
}
MelbourneDeveloper commented 3 years ago

This is a big problem. I'm trying to mock my client with Mockito, but it seems basically impossible. I'm going to have to put a wrapper around the logic just to mock the client when there is already a very thin wrapper around the client.

MelbourneDeveloper commented 3 years ago

@MarcinusX thanks so much! Your answer almost worked, but I had to change it to this and now it works perfectly!

class MockResponseFuture<T> extends Mock implements ResponseFuture<T> {
  final T value;

  MockResponseFuture(this.value);

  @override
  Future<S> then<S>(FutureOr<S> Function(T) onValue, {Function? onError}) =>
      Future.value(value).then(onValue, onError: onError);
}
inf0rmatix commented 1 year ago

Thanks @MelbourneDeveloper that saved me quite some time!