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

language/unsorted/flatten_test fails with dart2wasm-linux-chrome (but not with d8) #52631

Closed
osa1 opened this issue Jun 7, 2023 · 5 comments
Closed
Labels
area-dart2wasm Issues for the dart2wasm compiler.

Comments

@osa1
Copy link
Member

osa1 commented Jun 7, 2023

It looks like the runtime errors are not thrown synchronously, and with Chrome the test ends before throwing the exception.

I'm not sure if the test needs asyncStart/asyncEnd or this is a dart2wasm issue.

Minimal repro:

import 'dart:async';

class Derived<T> implements Future<T> {
  noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}

test() async {
  // flatten(Derived<int>) = int
  Future<int> x = (() async => new Derived<int>())();
}

main() {
  test();
}

This fails with a runtime error in d8, but passes in Chrome.

@osa1 osa1 added the area-dart2wasm Issues for the dart2wasm compiler. label Jun 7, 2023
@osa1
Copy link
Member Author

osa1 commented Jun 7, 2023

Another example of a similar failure is the test language/nnbd/static_errors/unchecked_use_of_nullable_test. Minimal repro:

void main() async {
  int? x;
  x.noSuchMethod(Invocation.method(#toString, []));
}

Currently this test fails as expected in Chrome because it fails with a Wasm trap. When we fix null.noSuchMethod to throw "no such method" (done in https://dart-review.googlesource.com/c/sdk/+/307800) it starts to fail in Chrome because the exception is not propagated to the test driver and test succeeds unexpectedly.

@osa1
Copy link
Member Author

osa1 commented Jun 14, 2023

One explanation why these tests pass with d8 but fail with Chrome can be that we currently use setTimeout for scheduling (this will change with https://dart-review.googlesource.com/c/sdk/+/305201), which in d8 does not wait before calling the callback. So some of the async callbacks will be called almost synchronously.

With https://dart-review.googlesource.com/c/sdk/+/305201 we override d8's setTimeout, but the new event loop implementation used in d8 waits for all tasks to complete, so these tests will still work as expected with d8 and fail with Chrome because the test driver we use in Chrome does not wait (and has no way to wait) for all events to complete.

@eernstg
Copy link
Member

eernstg commented Jun 14, 2023

Note that the compile-time error in 'language/unsorted/flatten_test.dart' isn't relevant for the behavior of the flatten function, so we might want to delete line 36, or we could rewrite it to use a local function with a declared return type rather than a function literal (like the one in line 35).

Cf. dart-lang/language#3148, which is an investigation of the compile-time error. Cf. https://dart-review.googlesource.com/309280 which will adjust the test.

@eernstg
Copy link
Member

eernstg commented Jun 15, 2023

Test landed (note that it was cleaned up quite extensively, but it still covers the same cases, and then some more). It was moved to $SDK/tests/language/async/divergent_flatten_test.dart.

@mkustermann
Copy link
Member

The language/async/divergent_flatten_test and language/nnbd/static_errors/unchecked_use_of_nullable_test are passing now.

The tests should be using asyncStart/asyncEnd which I'll do in cl/368581

The issue with test runner not recognizing async tests has been fixed some time ago.

copybara-service bot pushed a commit that referenced this issue Jun 1, 2024
In order for asynchronous tests to work on the web, they have to tell
the test runner that

* the test is async
* when the async test has finished running

This is done via calling asyncStart/asyncEnd.

Issue #52631
Issue #55865

Change-Id: I790960d7da708de4e36be9197ceb3e89f316a84e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/368581
Reviewed-by: Ömer Ağacan <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart2wasm Issues for the dart2wasm compiler.
Projects
None yet
Development

No branches or pull requests

3 participants