New async(inject()) test pattern does not handle unresolved promises; no mention of work around

[Yona-Appletree]

Steps to reproduce and a minimal demo of the problem

Write a test using async(inject()) (or the deprecated injectAsync) which returns a promise that is never fulfilled. Prior to #7735, the test would fail. It now passes.

See http://plnkr.co/edit/FTEe8DmlZGmFDQBDqcrv?p=preview for a trivial example of a test that should fail, but passes.

Current behavior

The given way of testing asynchronous code, async(inject()) and by extension, injectAsync, does not (and does not seem provide a way to) ensure that a test promise is completed.

Expected/desired behavior

A built-in method to ensure that a test will only pass if a particular promise is resolved. Perhaps simply by bringing back the old functionality of injectAsync and renaming it to something else.

Additionally, the change log (https://github.com/angular/angular/blob/master/CHANGELOG.md) should mention this important and breaking change (it does mention injectAsync -> async(inject()), but not the substantial change in behavior).

Other information

We have a number (~50) tests that rely the behavior of returning a promise that should resolve, or the test fails. After we upgraded to beta17, we started seeing false successes on tests. This mostly occurred because our business logic would call the mock HTTP service, but not with the correct parameters (e.g. http method or path) and our mock response code would thusly not return a response. Since all async methods had completed at that point, the test would erroneously pass.

[Yona-Appletree]

It seems that I may have spoken too soon. Am I right in thinking that the old behavior can be obtained simply by removing the async call and just using inject()?

[Yona-Appletree]

So, it looks like the above is correct: if you want to return a promise that should pass the test on resolution, you're free to do that. The only difference is that no check will be made to ensure that you do indeed return a promise. That seems fine.

It still would have been nice to see some mention in the changelog, but it may just be me not understanding the jasmine API well enough.

[kenjiqq]

Just ran into this myself, a simple test as passes successfully.

it('should fail', async(() => {
    const promise = new Promise(() => {});
    promise.then(() => {
        expect(false).toBe(true);
    });
  }));

Don't understand why more people have not mentioned this, it doesn't seem clear to me from the docs if this is supposed to work or not.

[gkalpak]

@kenborge, can you please open a new issue about that. (It is either something we need to fix or document better - I m not sure myself 😁)

[kenjiqq]

@gkalpak I created a new issue #20827

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}