build

build copied to clipboard

build_resolvers: 1.3.1 build_test: 0.10.11

See commit https://github.com/dart-lang/build/commit/b6fdcb0508c6786113ceda73ba17506dcb51718a

This is the failing test:

https://github.com/kevmoo/source_gen_test/blob/c1aff801cba9f46c27a1da854fbeed75fb7b2a1e/test/init_library_reader_test.dart#L54-L62

https://github.com/dart-lang/build/blob/master/build_test/lib/src/resolve_source.dart#L197 😢

https://github.com/dart-lang/build/blob/master/build_test/lib/src/resolve_source.dart#L197 😢

I tried just awaiting that – still has the problem!

Awaiting that fixes the issue but then the test has to be updated to expect the new exception type (which is the documented one, previously it threw a different one). I think it might be breaking for other use cases though to await there, I need to look into it a bit more.

cc @matanlurey

Ya if we awaited there we would block on the tearDown future being resolved so this would just hang tests that use that.

I don't really know what the correct solution is going to be here.

Reopening for more discussion. This change can cause some legitimate failures to get lost.

When I drop the .catchError here I can't repro any failure with the source_gen_test tests. @kevmoo - do you expect that something changed in those tests?

I'm not sure how much retaining these specific errors would help though. If the async error comes after the builder completes normally we'd still ignore those errors.

https://github.com/dart-lang/build/blob/2d36ee33d138edd35542b3f9633f7032a54f19fc/build/lib/src/builder/logging.dart#L28

@natebosch should we close this at this point?

I think #3253 and #3245 are examples of things that we can miss because of this design.

I found both of those examples while I was experimenting with migrating those tests to package:checks.

I think this is still a likely issue when using expect, but with checks we'd more likely get an unawaited_future, and the await resolves the issue.

I think that the complexity to solve this is not justified for continued safety with a library that has a foot-gun around async expectations. If we invest in async safety for package:matcher I'd first look at adding lints to use expectLater with AsyncMatcher which I think also would add enough safety for this problem.