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

Dart 2 SDK Tests vs flutter test --preview-dart-2 #15900

Closed
brianegan opened this issue Mar 24, 2018 · 11 comments
Closed

Dart 2 SDK Tests vs flutter test --preview-dart-2 #15900

brianegan opened this issue Mar 24, 2018 · 11 comments

Comments

@brianegan
Copy link
Contributor

brianegan commented Mar 24, 2018

I've been running into a funny issue updating the Redux library for Dart 2. Since Redux does not depend on Flutter, I generally run tests in a pure dart environment. When I run tests using the Dart 2 SDKs, all tests pass. When I run tests in a Flutter environment, I run into errors.

I'm not sure if these are accurate type errors that are no longer supported by Dart 2, or if there's some bug in Flutter, but it's a bit confusing since the tests pass using the Dart 2 SDK by itself.

Steps to Reproduce

  1. Clone the Flutter Architecture Samples repo (https://github.com/brianegan/flutter_architecture_samples/tree/redux-flutter-test/example/redux)
  2. Checkout the redux-flutter-test branch
  3. Navigate to the example/redux folder
  4. Run combine_test using the Dart SDK provided by flutter, everything works
  5. Run the tests using flutter test --preview-dart-2 and I get type errors

Logs

example/redux ‹redux-flutter-test*› » ~/lab/flutter/bin/cache/dart-sdk/bin/dart --version             
Dart VM version: 2.0.0-dev.28.0.flutter-0b4f01f759 (Tue Feb 20 17:53:06 2018 +0000) on "macos_x64"

example/redux ‹redux-flutter-test*› »  ~/lab/flutter/bin/cache/dart-sdk/bin/dart test/combine_test.dart
00:00 +0: Combining Reducers Type Safe Combinations are invoked when they match the Type of the dispatched action
00:00 +1: Combining Reducers Type Safe Combinations are not invoked if they do not handle the action type
00:00 +2: Combining Reducers Dynamic Combinations when two reducers are combined, each reducer is invoked.
00:00 +3: Typed Middleware are invoked based on the type of action they accept
00:00 +4: Typed Middleware are not invoked if they do not handle the Action and call the next piece of middleware in the chain
00:00 +5: All tests passed!


example/redux ‹redux-flutter-test*› » flutter test --preview-dart-2 test/combine_test.dart            
00:01 +0 -1: Combining Reducers Type Safe Combinations are invoked when they match the Type of the dispatched action [E]
  type '(String, TestAction1) => String' is not a subtype of type '(String, dynamic) => String'
  package:redux/src/utils.dart         combineTypedReducers.<fn>.<fn>
  dart:collection                      _Object&ListMixin^^#T0.fold
  package:redux/src/utils.dart 113:21  combineTypedReducers.<fn>
  package:redux/src/store.dart 218:21  Store._createReduceAndNotify.<fn>
  package:redux/src/store.dart 250:20  Store.dispatch
  test/combine_test.dart 24:15         main.<fn>.<fn>.<fn>
  
00:01 +2 -2: Typed Middleware are invoked based on the type of action they accept [E]                            
  type '(Store<String>, TestAction1, (dynamic) => void) => void' is not a subtype of type '(Store<String>, dynamic, (dynamic) => void) => void'
  package:redux/src/utils.dart         combineTypedMiddleware.<fn>.<fn>
  package:redux/src/store.dart 238:43  Store._createDispatchers.<fn>
  package:redux/src/store.dart 250:20  Store.dispatch
  test/combine_test.dart 94:13         main.<fn>.<fn>
  
00:01 +3 -2: Some tests failed. 

Run flutter analyze and attach any output of that command also.

Analyzing /Users/phillywiggins/lab/flutter_architecture_samples/example/redux...
No issues found!
Ran in 6.6s

Flutter Doctor

[✓] Flutter (Channel beta, v0.1.5, on Mac OS X 10.13.3 17D102, locale en-US)
    • Flutter version 0.1.5 at /Users/phillywiggins/lab/flutter
    • Framework revision 3ea4d06340 (4 weeks ago), 2018-02-22 11:12:39 -0800
    • Engine revision ead227f118
    • Dart version 2.0.0-dev.28.0.flutter-0b4f01f759

[✓] Android toolchain - develop for Android devices (Android SDK 27.0.2)
    • Android SDK at /Users/phillywiggins/Library/Android/sdk
    • Android NDK at /Users/phillywiggins/Library/Android/sdk/ndk-bundle
    • Platform android-27, build-tools 27.0.2
    • ANDROID_HOME = /Users/phillywiggins/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-915-b08)

[✓] iOS toolchain - develop for iOS devices (Xcode 9.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 9.2, Build version 9C40b
    • ios-deploy 1.9.2
    • CocoaPods version 1.3.1

[✓] Android Studio (version 3.0)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-915-b08)

[✓] IntelliJ IDEA Community Edition (version 2017.3.5)
    • Flutter plugin version 22.2.2
    • Dart plugin version 173.4700

[✓] VS Code (version 1.21.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Dart Code extension version 2.8.2

[✓] Connected devices (1 available)
    • Android SDK built for x86 • emulator-5554 • android-x86 • Android 8.0.0 (API 26) (emulator)

• No issues found!
@brianegan brianegan changed the title Dart 2 SDK Tests vs Flutter Test --preview-dart-2 Dart 2 SDK Tests vs flutter test --preview-dart-2 Mar 24, 2018
@DanTup
Copy link
Contributor

DanTup commented Mar 25, 2018

I'm not sure if these are accurate type errors that are no longer supported by Dart 2, or if there's some bug in Flutter, but it's a bit confusing since the tests pass using the Dart 2 SDK by itself.

Have you tried running with --preview-dart-2 directly on the Dart VM? (~/lab/flutter/bin/cache/dart-sdk/bin/dart --preview-dart-2 test/combine_test.dart)? This might confirm whether it's just Dart 2 changes versus something in Flutter?

@brianegan
Copy link
Contributor Author

@DanTup Good call -- I should have included that run as well. I get the same results:

example/redux ‹redux-flutter-test› »  ~/lab/flutter/bin/cache/dart-sdk/bin/dart --preview-dart-2 test/combine_test.dart     
00:00 +0: Combining Reducers Type Safe Combinations are invoked when they match the Type of the dispatched action
00:00 +1: Combining Reducers Type Safe Combinations are not invoked if they do not handle the action type
00:00 +2: Combining Reducers Dynamic Combinations when two reducers are combined, each reducer is invoked.
00:00 +3: Typed Middleware are invoked based on the type of action they accept
00:00 +4: Typed Middleware are not invoked if they do not handle the Action and call the next piece of middleware in the chain
00:00 +5: All tests passed!

@brianegan
Copy link
Contributor Author

Sorry to ping on this one again -- trying to get my stuff Dart 2 ready! Does anyone have any insights into this issue?

@DanTup
Copy link
Contributor

DanTup commented Mar 29, 2018

@brianegan I had a go at reproing this, but failed to restore packages:

dantup-macbookpro:redux dantup$ flutter packages get
Running "flutter packages get" in redux...                       
Incompatible version constraints on intl:
- flutter_architecture_samples 0.0.1 depends on version 0.15.1
- flutter_driver 0.0.0 depends on version 0.15.4
pub get failed (1)

I'm likely on a later flutter. Have you tried on either the new beta update this week, or on master to see if it might already fixed? I think there have been a lot of fixes since the first beta. (I don't think you use it, but in VS Code you can check both master and beta out and quickly switch between them from the status bar).

@brianegan
Copy link
Contributor Author

brianegan commented Mar 29, 2018

@DanTup I ran into this issue on master as well, but worth giving it another shot. Thanks for the help!

@DanTup
Copy link
Contributor

DanTup commented Mar 29, 2018

FWIW, I suspect the error is the correct behaviour and the Dart VM is incorrect (or outdated). If you define a function as taking dynamic but then passed it something that requires a specific type, it'd be a runtime error if you called it passing something else.

@brianegan
Copy link
Contributor Author

brianegan commented Mar 29, 2018

@DanTup Yep, that's definitely what I'm trying to suss out... basically, I think I have the wrong base types going on. I was using dynamic in Dart 1, and need a replacement for Dart 2.

The exact function that's throwing the error is this one:

// User defines the type of Action they want to handle
class ReducerBinding<State, Action> { /* Omitted for brevity */ }

// Function takes in a list of ReducerBindings with different types of actions handled.
// It returns a function of the correct signature: (State, dynamic) => State
//
// I've tried using both ReducerBinding<State, dynamic> and ReducerBinding<State, Object>
// but neither works. I think this is where the problem lies: I need to use the proper
// base type for all Objects for Dart 2.
Reducer<State> combineTypedReducers<State>(
    List<ReducerBinding<State, dynamic>> bindings) {
  return (State state, dynamic action) {
    return bindings.fold(state, (currentState, binder) {
      if (binder.handlesAction(action)) {
        return binder.reducer(state, action);
      } else {
        return currentState;
      }
    });
  };
}

@DanTup
Copy link
Contributor

DanTup commented Mar 29, 2018

I don't know a lot about how this works, but can you not use generic type args so the types all match up everywhere? (it if works, you'll get extra type safety you didn't have with dynamic)

@brianegan
Copy link
Contributor Author

brianegan commented Mar 29, 2018

In this case, that's not possible. An action needs to be of any type (dynamic or Object or some base Type). This allows both the user and 3rd party libraries to dispatch Actions.

I think it boils down to this: I'm wondering why ReducerBinding<State, Action> can't be a subtype of ReducerBinding<State, Object> or ReducerBinding<State, dynamic>. Is this a new covariance / contravariance rule in Dart 2? Can this case be satisfied in any way?

@brianegan
Copy link
Contributor Author

brianegan commented Mar 29, 2018

Ok, got it working by shuffling some code around, but still curious if this might be a bug. Thanks for the help @DanTup :)

The problem in the end turned out to be this line:

binder.reducer(state, action). Since the action is dynamic, and the binder.reducer has a Typed action param, they didn't match. What made it confusing is I had tried explicitly casting the action in a previous attempt to the correct type, using action as CorrectType, and it still failed for me, so I assumed that wasn't the issue.

However, if I pass the action into a function that accepts dynamic, and casts inside an if statement there, all works.

State reduce(State state, dynamic action) {
    if (action is Action) {
      return reducer(state, action);
    } else {
      return state;
    }
  }

Gonna close this one out and see if I can create a simplified test case for the Dart 2 Repo.

@github-actions
Copy link

github-actions bot commented Sep 3, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants