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

flutter_driver pauses all isolates at their startup (even ones started from compute()), rather than only pausing initial isolate #24703

Closed
faustinoribeiro opened this issue Nov 25, 2018 · 62 comments

Comments

@faustinoribeiro
Copy link

@faustinoribeiro faustinoribeiro commented Nov 25, 2018

Isolates spawned using "compute" don't execute properly while executed in an integration test which in turn results in incorrect app behavior.
How to reproduce:

  1. add:
import 'package:flutter/foundation.dart';
  1. add function:
String _test(String s) {
  return s;
}
  1. modify _incrementCounter function:
  void _incrementCounter() async {
    final s = await compute(_test, 'hello');
    print(s);

    setState(() {
      _counter++;
    });
  }

The app works properly when executing flutter run, but the integration test fails (flutter drive --target=test_driver/app.dart)

@zoechi
Copy link
Contributor

@zoechi zoechi commented Nov 26, 2018

Tests use https://pub.dartlang.org/packages/fake_async by default.
Try with https://docs.flutter.io/flutter/flutter_test/WidgetTester/runAsync.html to get real async behavior.

Please consider asking support questions in one of the other channels listed at http://flutter.io/support .

@faustinoribeiro
Copy link
Author

@faustinoribeiro faustinoribeiro commented Nov 26, 2018

Thanks for the quick response.
I will go to support first next time instead of wrongly assuming it is a bug.
It would be nice to have a "gotcha" section in the documentation about cases like this one.

Keep the great work, Flutter is the first mobile framework I really enjoy working with. I have with 3 other frameworks in the past and it was rarely smooth as Flutter.

@zoechi zoechi added this to the Stretch Goals milestone Nov 26, 2018
@faustinoribeiro
Copy link
Author

@faustinoribeiro faustinoribeiro commented Nov 26, 2018

Getting back to this issue (please let me know if I should move this to support).
The suggested solution using runAsync applies to widgets testing if I understand correctly.
As I mentioned in my initial comment, I am in early stage of integration testing with flutter_driver.
I am probably missing the obvious, but how do I use runAsync (or something else) to get "compute" isolates to execute properly in that context?

@zoechi
Copy link
Contributor

@zoechi zoechi commented Nov 26, 2018

I don't know if isolates are supposed to work in this setup but I assume so (haven't tried myself).

#21368 shows a code snippet.
It's about wrapping your test code with runAsync.

See also #5728

@faustinoribeiro
Copy link
Author

@faustinoribeiro faustinoribeiro commented Nov 27, 2018

I tried to follow the examples you provide by wrapping my app with runAsync with the hope that the integration tests run smoothly, but the app doesn't even launch. I don't have time to run further tests at this time, so I will come back to this in a couple of weeks. Thanks for the help.

@zoechi
Copy link
Contributor

@zoechi zoechi commented Nov 27, 2018

@Hixie who might know if this is supposed to work?
I tried GitHub search but couldn't find unit tests for compute() that could be used as example.

@jeroentrappers
Copy link

@jeroentrappers jeroentrappers commented Nov 28, 2018

Could you give an example of how to wrap the app under test in runAsync ?

@faustinoribeiro
Copy link
Author

@faustinoribeiro faustinoribeiro commented Nov 29, 2018

This is what I tried:

import 'package:flutter_driver/driver_extension.dart';
import 'package:flutter_test/flutter_test.dart';
import '../lib/my_app.dart';

void main() {
    enableFlutterDriverExtension();

    testWidgets(
        'app test',
        (WidgetTester tester) async {
            final app = MyApp();

            await tester.runAsync(() async {
                await tester.pumpWidget(app);

                await tester.idle();
            });
        }
    );
}

@Hixie
Copy link
Member

@Hixie Hixie commented Nov 30, 2018

Generally driver tests are not expected to use flutter_test. The device side is expected to just be the app, and the driver side is expected to be a command-line app. Using testWidgets with driver tests is going to cause a world of pain.

@faustinoribeiro
Copy link
Author

@faustinoribeiro faustinoribeiro commented Dec 1, 2018

Yes, I guessed that much, but just gave a shot.
This is still surprising that driver tests would make isolates fail (assuming I am not doing something really wrong). How is that even possible? I might need (once I get back to working on integration tests) to use some type of flag to not use compute() while testing. This would however mean that I am not doing proper testing.

@ddikman
Copy link

@ddikman ddikman commented Jan 10, 2019

I am experiencing this as well in my application when attempting to test with the driver. My application loads assets at startup and ends up hanging on a call to AssetBundle.loadString which has a compute call.

The code is open source so I can share that but if it helps I'm happy to reproduce it in an isolated repo.

@jaripekkala
Copy link

@jaripekkala jaripekkala commented Mar 15, 2019

Meanwhile using a workaround that probably doesn't suit for everybody's needs

import 'package:flutter/foundation.dart' as flutter show compute;
import 'package:flutter/foundation.dart' show ComputeCallback;

Future<R> compute<Q, R>(ComputeCallback<Q, R> callback, Q message) async {
  if (isInDebugMode) {
    return callback(message);
  }

  return await flutter.compute(callback, message);
}

@faustinoribeiro
Copy link
Author

@faustinoribeiro faustinoribeiro commented Mar 15, 2019

I implemented a similar approach when trying to figure out why my tests were failing. It works, but it's far from perfect though because you're not testing your app as it will be shipped. It's acceptable if you have a couple of simple compute operations, but not so much when you have a lot more of those in critical places (like I currently have in many of my app API calls).

@MariaMelnik
Copy link
Contributor

@MariaMelnik MariaMelnik commented Mar 18, 2019

Seems like there isn't any workaround for Isolate with async function because of #25890
I have case with authentication in Isolate (with async function inside) and it turns out I don't have a way to login to my app during integration test.

@The-Redhat
Copy link

@The-Redhat The-Redhat commented Apr 6, 2019

@Hixie Hey, is anything planned to resolve this issue ?

@The-Redhat
Copy link

@The-Redhat The-Redhat commented May 1, 2019

Hey @zoechi I think this issue should be added to the Goals milestone. It prevents me from adding integration tests to my which is really annoying for a production app. Thanks for your help!

@eikob
Copy link

@eikob eikob commented May 29, 2019

This is really annoying and came totally unexpected when using the screenshot package and the Future from DefaultAssetBundle.loadString() just never happened to return. Using .load() instead and manually doing the conversion fixed it, but no way to find out why without looking into the source code.

Also, this can break basically anything, as we have no idea what calls the compute() function under the hood.

@brmaleo
Copy link

@brmaleo brmaleo commented Jun 24, 2019

I would like to clarify, the problem appears when using FlutterDriver itself. Not necessarily when running tests with package:flutter_test or package:test.

Even when running the following code directly...

void main() async
{
  FlutterDriver driver = await FlutterDriver.connect();

  await driver.waitFor(find.byValueKey("AssetLoadingButton"));
  await driver.tap(find.byValueKey("AssetLoadingButton"));
}

... the AssetBundle.loadString inside the app will hang indefinitely (if the string asset is larger than 10*1024 bytes in size).

@eikob is absolutely right. It is only a matter of time when a particular package accidentally breaks the UI automation.

If this behavior of FlutterDriver is "a feature, not an issue", I would like to see at least a universal workaround. The official Flutter docs does not mention anything about running applications "really asynchronously" in the right way.

@Kiruel
Copy link

@Kiruel Kiruel commented Jun 26, 2019

Anynews on this issue ?

@The-Redhat
Copy link

@The-Redhat The-Redhat commented Jun 26, 2019

After looking at the dart VM Observatory I found out, that the isolate is paused at start. When you manually start the isolate, it works perfectly. Maybe that helps figuring out the issue.

@Sorunome
Copy link

@Sorunome Sorunome commented Jul 17, 2020

@vishna Your fix is working great. however the gist doesn't appear to have any license, making it hard to use in some projects. Would you mind to add a license to it, please?

@vishna
Copy link

@vishna vishna commented Jul 21, 2020

@vishna Your fix is working great. however the gist doesn't appear to have any license, making it hard to use in some projects. Would you mind to add a license to it, please?

added MIT License

@speaking-in-code
Copy link
Contributor

@speaking-in-code speaking-in-code commented Jul 27, 2020

My fix for this in #61841 was reverted with #62239. So this bug is still open.

If anyone has tips on how to run the devicelab tests, I'm happy to try to debug the issues so that this fix can land.

@acoutts
Copy link

@acoutts acoutts commented Aug 4, 2020

When I try to run flutter drive --target=test_driver/app.dart --flavor sandbox -d 9A311FFAZ00C03 --profile --cache-sksl --write-sksl-on-exit sksl_cache/android.sksl.json on android, the test never runs. The app is installed and the splash screen shows but the dart isolate never starts.

...
Running Gradle task 'assembleSandboxProfile'... Done              109.4s
✓ Built build/app/outputs/flutter-apk/app-sandbox-profile.apk (65.7MB).
Installing build/app/outputs/flutter-apk/app.apk...                 5.1s
I/flutter (26379): Observatory listening on http://127.0.0.1:39715/yP9wvTmO1Pw=/
00:00 +0: Bottlepay app SkSL warmup (setUpAll)

Connecting driver

VMServiceFlutterDriver: Connecting to Flutter application at http://127.0.0.1:62204/yP9wvTmO1Pw=/
VMServiceFlutterDriver: Isolate found with number: 3846210052022527
VMServiceFlutterDriver: Unknown pause event type VMNoneEvent. Assuming application is ready.
E/FlutterFcmService(26379): Fatal: failed to find callback
VMServiceFlutterDriver: Flutter Driver extension is taking a long time to become available. Ensure your test app (often "lib/main.dart") imports "package:flutter_driver/driver_extension.dart" and calls enableFlutterDriverExtension() as the first call in main().

It then hangs here indefinitely. Is this related?

According to debug printing, it is hanging on driver = await FlutterDriver.connect();.

group('Bottlepay app SkSL warmup', () {
    FlutterDriver driver;
    IsolatesWorkaround workaround;

    // Connect to the Flutter driver before running any tests.
    setUpAll(() async {
      print('Connecting driver');
      driver = await FlutterDriver.connect();
      print('Driver connected');

      if (driver.appIsolate.isPaused) {
        print('Waiting for IsolatesWorkaround to resume isolates');
        workaround = IsolatesWorkaround(driver);
        await workaround.resumeIsolates();
        print('Isolates resumed');
      }

      print('Waiting for first frame to rasterize');
      await driver.waitUntilFirstFrameRasterized();
      print('First frame rasterized');
    });

    // Close the connection to the driver after the tests have completed.
    tearDownAll(() async {
      if (driver != null) {
        await driver.close();
        await workaround?.tearDown();
      }
    });

I am unable to run driver tests on android for both a simulator and physical device because of this. On the simulator it freezes on await driver.waitUntilFirstFrameRasterized();.

@speaking-in-code
Copy link
Contributor

@speaking-in-code speaking-in-code commented Aug 6, 2020

I got back to this and reproduced the problem.

I suspect https://github.com/flutter/gallery/blob/master/test_driver/isolates_workaround.dart is related. Maybe a conflict between the two fixes for the same bug?

@speaking-in-code
Copy link
Contributor

@speaking-in-code speaking-in-code commented Aug 6, 2020

What do folks think of this approach to fixing the issue?

  1. fix flutter/gallery integration test isolate_workaround.dart to be OK with something else unpausing the isolate. Release that change.
  2. fix flutter driver to unpause isolates, as in #61841. Verify that the devicelab tests pass this time. Release that change.
  3. celebrate. Bug is fixed.
  4. wait a while for older versions of flutter driver to go out of circulation.
  5. delete flutter/gallery isolate_workaround.dart, since it's no longer needed with new flutter versions.

@guidezpl
Copy link
Member

@guidezpl guidezpl commented Aug 6, 2020

  1. and 5. will be complete with flutter/gallery#256

The gallery integration tests which are run from flutter/flutter are pinned to the commit in
https://github.com/flutter/flutter/blob/master/dev/devicelab/lib/versions/gallery.dart. The reland PR for 2. will have to update that hash.

@tvolkert
Copy link
Contributor

@tvolkert tvolkert commented Sep 11, 2020

Looks like this is getting reverted again due to Google test failures, so re-opening.

@speaking-in-code see also b/77244607

/cc @rmacnak-google since we were discussing this back in April 2018. At the time, we mused about the ability to send a command to the vm service saying "change the vm behavior such that we don't start new isolates paused anymore".

@HerrNiklasRaab
Copy link

@HerrNiklasRaab HerrNiklasRaab commented Oct 26, 2020

@tvolkert Is this now really fixed in dev or was it again reverted?

@tvolkert
Copy link
Contributor

@tvolkert tvolkert commented Oct 26, 2020

@HerrNiklasRaab this was re-landed and re-closed in #65703 (7a4d8e1), which was included in 1.24.0-3.0.pre, currently on dev.

@lukepighetti
Copy link

@lukepighetti lukepighetti commented Jan 1, 2021

I have run into a situation that may be related.

I have a 1.7MB json asset that I am loading as a string with rootBundle.loadString(path) and it's making my tests (we're not using driver) hang indefinitely. A 4kb json file going through the exact same code is working flawlessly. The only difference appears to be the size of the asset. I have tried running this on beta and dev and no change.

I tried loading the string outside of a compute but that didn't resolve the issue.

final data = await rootBundle.load(path);
final string = utf8.decode(data.buffer.asUint8List());

Is loading large string assets causing non-driver tests to hang indefinitely a known issue?

EDIT:

If I swap testWidgets for test and then fire TestWidgetsFlutterBinding.ensureInitialized(); it works. Is this working as expected? Is this even related to this issue?

@tvolkert
Copy link
Contributor

@tvolkert tvolkert commented Jan 2, 2021

@lukepighetti yeah that sounds like it's related to this issue.

  1. We have code in the framework that, when loading a string asset, spawns an isolate to do the UTF8 decoding if the raw bytes is over 50K in length. This was done because UTF8 decoding can be expensive, so for loading very large strings (e.g. the Flutter LICENSE file), we choose not to block the main isolate.
  2. The Dart VM, when told to start its isolates in a paused state (--pause_isolates_on_start), will do so for all isolates as long as the VM is running -- even after the VM receives the message to un-pause all isolates (which is what this bug is filed about)
  3. Driver tests use --pause_isolates_on_start, and the host-based driver code un-pauses all isolates as the driver extension comes up.

@passsy
Copy link
Contributor

@passsy passsy commented May 6, 2021

I'm using this wrapper to allow my own compute calls to execute during tests

/// Because Isolates are started "paused" during tests (--start-paused) and don't execute at all, this function 
/// wraps [compute] and executes the computation on the same [Isolate].
Future<R> testableCompute<Q, R>(ComputeCallback<Q, R> callback, Q message, {String? debugLabel}) async {
  // During tests, don't start a new isolate, instead execute the computation on the same isolate
  if (!kIsWeb && Platform.environment.containsKey('FLUTTER_TEST')) {
    // fake Isolate.spawn delay
    await Future.delayed(Duration(milliseconds: 500));
    return await callback(message);
  }
  return await compute(callback, message, debugLabel: debugLabel);
}

@github-actions
Copy link

@github-actions github-actions bot commented Aug 2, 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 Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.