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

Should throwsA mark tests pending when the value is a Future? #1670

Closed
natebosch opened this issue Feb 15, 2022 · 4 comments
Closed

Should throwsA mark tests pending when the value is a Future? #1670

natebosch opened this issue Feb 15, 2022 · 4 comments

Comments

@natebosch
Copy link
Member

It's possible to lose test failures if you use throwsA against a Future which does not complete until after all other tests are done.

If we see a Future that we expect to throw, should we mark the test pending until it completes?

natebosch added a commit to dart-lang/build that referenced this issue Feb 15, 2022
This test should be failing but the error is lost because `throwsA` does
not force the test to wait until the condition is checked.
dart-lang/test#1670

Fix the pattern to use `await expectLater` instead of `await expect` so
the failure will show up. Skip the test for now.
@jakemac53
Copy link
Contributor

Seems like we should imo, yes. I always thought it worked that way...

@natebosch
Copy link
Member Author

hmm, I may be wrong about the root cause of that test not failing... I think the intention is this async work should be captured by

final outstandingWork = test.markPending();

I'm not sure why this failure is slipping through

@natebosch
Copy link
Member Author

Ah, this bug is related to our use of zones in the build system.

The root cause is that we're running test expectations in a nested zone which has it's own uncaught error handler. This test failure is getting ignored due to dart-lang/build#2599

I can't find the conversation now, but I think at one point we had discussed making fail() an API on TestHandle. If we do that then we can allow communicating this failure more directly without relying on the uncaught async error behavior.

@natebosch
Copy link
Member Author

Closing in favor of #1671

natebosch added a commit to dart-lang/build that referenced this issue Feb 16, 2022
This test should be failing but the error is lost because of the way the
async failure interacts with the `runBuilder` error handling zone.
dart-lang/test#1670

Use `await expectLater` instead of `await expect` to ensure the error is
not lost.

Change the setup for the test to include two imports instead of 1 import
and a local class definition. Move the usage to an annotation so that it
is easy to find from the library using it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants