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

Generate Dart "testable" code for Flutter testability #1028

Closed
Hsilgos opened this issue Aug 5, 2021 · 3 comments · Fixed by #1039 or #1048
Closed

Generate Dart "testable" code for Flutter testability #1028

Hsilgos opened this issue Aug 5, 2021 · 3 comments · Fixed by #1039 or #1048
Assignees
Labels
dart enhancement New feature or request

Comments

@Hsilgos
Copy link
Contributor

Hsilgos commented Aug 5, 2021

Flutter testing

Flutter unit tests are usually ran on host. Also it's important to disconnect business logic from generated interface to avoid side effects. Following options were investigated.

Dependency injection

There are few external dart libraries which allow to inject dependencies, for example get_it
Unfortunately they require to write or adopt existing code in specific way.

There is no way to replace (inject) package in tests with other package (for example with stubs) like it can be done for Java.

Conditional import

Dart 1.x supported conditional import based on specified compilation flags.
Dart 2.x is not so flexible and supports only limited conditional import based on presence of platform libraries dart.library.xxx, like

import "src/default.dart" 
  if (dart.library.io) "src/io.dart"
  if (dart.library.html) "src/web.dart"
  as impl;

I haven't found a way to use this approach to make conditional imports only for unit tests.

Override lookupFunction

Since all FFI functions are resolved with nativeLibrary.lookupFunction, this lookupFunction (or wrapper around this function) can be overridden and return mocked functions. Such approach could allow to not touch existing code, but provide generated code as extra plugin for testing which replaces __lib.nativeLibrary with overridden version.

The problem here is to return test data from such mocked functions. The functions work with data which is prepared for ffi call, for example Pointer<Void> instead of usual types. This Pointer<Void> must be converted back to the instance of type, perhaps with call to existing generated converter function.

It looks very costly to generate the code which allows to mock functions in convenient way.

TBD

Factory methods

The common approach is to call factory method to select real implementation or mocking runtime. In unit tests those factory methods must be replaced during fixture setup.

abstract class Foo {
    factory Foo(A a, B b, C c) => _factoryMake()

    static void doSomething() => _factoryDoSomething()

    static Foo? get sharedInstance => _factorySharedInstance();

    static Function(A,B,C) _factoryMake = Foo$Impl.make
    static Function() _factoryDoSomething = Foo$Impl.doSomething
    static Function() _factorySharedInstance = Foo$Impl.sharedInstance
}

class Foo$Impl extends __lib.NativeBase implements Foo {
    Foo$Impl.make(A a, B b, C c) {
    }

    static SDKNativeEngine? get sharedInstance {
    }

    static void doSomething() {
    }
...
}

Prototype class

Approach with prototype class looks easier and most convenient.

The requirements after all must be the following.

  • The generated code should contain all necessary mechanisms for mocking, no additional flag (i.e. stub) is necessary
  • Enums shouldn't be changed
  • Structs without calls to ffi also shouldn't be changed
  • Classes and structs with methods which are calling ffi must be changes like described below.

Each such class must contain static variable $prototype. This static variable must be initialized with instance of the $Impl class. All calls to constructors, static methods, getters, setters must be performed via $prototype

class Foo {
    factory Foo() => $prototype.make()

    static void doSomething() => $prototype.doSomething()
    static Foo? get sharedInstance => $prototype.sharedInstance;
    set sharedInstance(Foo? value) { $prototype.sharedInstance = value; }

    void someMethod()

    static var $prototype = Foo$Impl()
}

class Foo$Impl extends __lib.NativeBase implements Foo {
    Foo$Impl.make(A a, B b, C c) {
    }

    static SDKNativeEngine? get sharedInstance {
    }

    static void doSomething() {
    }

Example of usage:

Foo.$prototype = MockFoo();

class MockFoo implements Foo$Impl {
  MockFoo();

  @override
  void someMethod() {}
}
@DanielKamkha DanielKamkha changed the title Testable Flutter Add an option to generate Dart stubs for Flutter testability Aug 9, 2021
@DanielKamkha DanielKamkha added dart enhancement New feature or request labels Aug 9, 2021
@DanielKamkha
Copy link
Contributor

I think that functions that are static in Foo should be non-static in Foo$Impl.

@DanielKamkha DanielKamkha changed the title Add an option to generate Dart stubs for Flutter testability Add an option to generate Dart "testable" code for Flutter testability Aug 11, 2021
@DanielKamkha DanielKamkha self-assigned this Aug 12, 2021
DanielKamkha added a commit that referenced this issue Aug 12, 2021
Dart "-stubs" testable mode never really worked. The new testing approach is to
generate "$prototype" field for mocking in the production version and mark it as
"visible for testing".

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 12, 2021
Dart "-stubs" testable mode never really worked. The new testing approach is to
generate "$prototype" field for mocking in the production version and mark it as
"visible for testing".

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
@DanielKamkha DanielKamkha changed the title Add an option to generate Dart "testable" code for Flutter testability Generate Dart "testable" code for Flutter testability Aug 16, 2021
DanielKamkha added a commit that referenced this issue Aug 16, 2021
Updated Dart templates to expose a `$prototype` field from types with static
functions/propertoes to allow for mocking of these static functions/properties.

No changes to functional tests are required. Smoke tests are updated in a
separate commit.

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 16, 2021
Updated Dart templates to expose a `$prototype` field from types with static
functions/propertoes to allow for mocking of these static functions/properties.

No changes to functional tests are required. Smoke tests are updated in a
separate commit.

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 16, 2021
Updated Dart templates to expose a `$prototype` field from types with static
functions/propertoes to allow for mocking of these static functions/properties.

No changes to functional tests are required. Smoke tests are updated in a
separate commit.

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 16, 2021
Resolves: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 16, 2021
Updated Dart templates to expose a `$prototype` field from types with static
functions/propertoes to allow for mocking of these static functions/properties.

No changes to functional tests are required. Smoke tests are updated in a
separate commit.

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 16, 2021
Resolves: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 16, 2021
Dart "-stubs" testable mode never really worked. The new testing approach is to
generate "$prototype" field for mocking in the production version and mark it as
"visible for testing".

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 16, 2021
Updated Dart templates to expose a `$prototype` field from types with static
functions/propertoes to allow for mocking of these static functions/properties.

No changes to functional tests are required. Smoke tests are updated in a
separate commit.

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 16, 2021
Resolves: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 16, 2021
* Make generated Dart code testable/mockable

Updated Dart templates to expose a `$prototype` field from types with static
functions/propertoes to allow for mocking of these static functions/properties.

No changes to functional tests are required. Smoke tests are updated in a
separate commit.

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>

* Update smoke tests for Dart `$prototype` change

Resolves: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
@Hsilgos
Copy link
Contributor Author

Hsilgos commented Aug 18, 2021

There is a problem with structs with default constructors which may call ffi. The class seems unmockable now. I propose to move constructor to $Impl as well. May be it make sense to do the same for all classes, generated code must look like:

class Foo {
  final Bar bar1;
  final Bar bar2;

  const Foo._(this.bar1, this.bar2);

  factory Foo(Bar bar1, Bar bar2) => $prototype.$init(bar1, bar2);

  static Foo? staticMethod(List<Bar> bars) => $prototype.staticMethod(bars);

  bool nonStaticMethod(Foo foo) {
    /// Call to ffi
  }
}

@visibleForTesting
class Foo$Impl {
  Foo $init(Bar bar1, Bar bar2) {
    return Foo._(bar1, bar2);
  }
  Foo? staticMethod(List<Bar> bars) {
    /// call to ffi
  }
}

@Hsilgos Hsilgos reopened this Aug 18, 2021
@DanielKamkha DanielKamkha removed their assignment Aug 18, 2021
@DanielKamkha DanielKamkha self-assigned this Aug 18, 2021
@DanielKamkha
Copy link
Contributor

"Custom" constructors are already redirected for structs. Only "field-based" constructors need this additional treatment.

As classes only ever have "custom" ctors, nothing needs to be done for them.

DanielKamkha added a commit that referenced this issue Aug 19, 2021
Updated DartStruct template to improve testability:
* updated instance methods to be redirects to `$prototype`, similarly to how
it's done already for static methods.
* removed `_copy()` constructor as it was an unnecessary level of indirection
that also impeded testability.

Smoke tests are updated in a separate commit.

See: #1028
DanielKamkha added a commit that referenced this issue Aug 19, 2021
Updated DartStruct template to improve testability:
* updated instance methods to be redirects to `$prototype`, similarly to how
it's done already for static methods.
* removed `_copy()` constructor as it was an unnecessary level of indirection
that also impeded testability.

Updated smoke tests. Functional tests don't need to be updated because there is
no change in user-facing functionality.

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 19, 2021
Updated DartStruct template to improve testability:
* updated instance methods to be redirects to `$prototype`, similarly to how
it's done already for static methods.
* removed `_copy()` constructor as it was an unnecessary level of indirection
that also impeded testability.

Updated smoke tests. Functional tests don't need to be updated because there is
no change in user-facing functionality.

Resolves: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 20, 2021
Updated DartStruct template to remove `_copy()` constructor. It was an
unnecessary level of indirection that also impeded testability.

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 20, 2021
Updated DartStruct template to remove `_copy()` constructor. It was an
unnecessary level of indirection that also impeded testability.

See: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 20, 2021
Updated DartStruct template to improve testability:
* updated instance methods to be redirects to `$prototype`, similarly to how
it's done already for static methods.
* removed `_copy()` constructor as it was an unnecessary level of indirection
that also impeded testability.

Updated smoke tests. Functional tests don't need to be updated because there is
no change in user-facing functionality.

Resolves: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 23, 2021
Updated DartStruct template to improve testability: updated instance methods to
be redirects to `$prototype`, similarly to how it's done already for static
methods.

Updated smoke tests. Functional tests don't need to be updated because there is
no change in user-facing functionality.

Resolves: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
DanielKamkha added a commit that referenced this issue Aug 26, 2021
Updated DartStruct template to improve testability: updated instance methods to
be redirects to `$prototype`, similarly to how it's done already for static
methods.

Updated smoke tests. Functional tests don't need to be updated because there is
no change in user-facing functionality.

Resolves: #1028
Signed-off-by: Daniel Kamkha <daniel.kamkha@here.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dart enhancement New feature or request
Projects
None yet
2 participants