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

Jest + @firebase/rules-unit-testing has unstopped asynchronous operations #4884

Open
Francesco-Lanciana opened this issue May 10, 2021 · 12 comments
Labels

Comments

@Francesco-Lanciana
Copy link

[REQUIRED] Describe your environment

  • Operating System version: macOS Big Sur (Version 11.2.3)
  • Browser version: N/A
  • Firebase SDK version: 9.7.0
  • Firebase Product: firestore

[REQUIRED] Describe the problem

When using @firebase/rules-unit-testing and jest the tests will not exit cleanly and you will get the warning:
Jest did not exit one second after the test run has completed.. Deleting the apps after the tests have finished does not help.

Steps to reproduce:

  1. Run the firestore emulator using firebase emulators:start
  2. Make sure jest is installed (I'm using version 26.6.3). In my case I'm also using ts-jest and @types/jest to write my tests using typescript.
  3. Create a basic test that attempts to get a document from the Firestore DB
  4. Run the tests using the jest command and you will get the following warning:
Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

--detectOpenHandles does not help at all, it just hangs instead of exiting.
I tried to delete every app after each test in order to close any asynchronous calls but that also didn't help.

Relevant Code:

import * as firebase from "@firebase/rules-unit-testing";

describe("Security rules", () => {
    afterEach(async () => {
        await Promise.all(firebase.apps().map((app) => app.delete()));
    });

    it("Can read stuff", async () => {
        const app = firebase.initializeTestApp({ projectId: "whatever" });
        const db = app.firestore();
        const testDoc = db.collection("private").doc("a_doc");
        await firebase.assertFails(testDoc.get());
    });
});
@looptheloop88 looptheloop88 added the testing-sdk testing with emulator label May 10, 2021
@daniel-sudz
Copy link

daniel-sudz commented May 10, 2021

for a workaround, this is what I have been using in my database tests. It's been working for me:

    "test": "jest --detectOpenHandles --forceExit",
    "ci": "firebase emulators:exec --only firestore \"yarn test\""

The forceExit flag causes jest to exit even with pending promises

@looptheloop88
Copy link

@Francesco-Lanciana, thanks for the report. Would you mind creating and sharing a sample project that I can run locally to replicate the issue?

@Francesco-Lanciana
Copy link
Author

Hey @looptheloop88 here is a minimum sample project that replicates the issue. Looking forward to hearing how you go with it. I'm very keen to get this bug fixed!

@looptheloop88
Copy link

Hi @Francesco-Lanciana, thanks for providing the MVCE. I appreciate it.

I was able to reproduce the warning and informed our engineers about this. In the meantime, you can get rid of the warning using the workaround provided by @daniel-sudz.

@akauppi
Copy link

akauppi commented May 19, 2021

@Francesco-Lanciana Since both Jest and Firebase are mentioned, I'll use this opportunity.

I have today released 0.0.3 beta of a tool that intends to make testing Firebase backends - Security Rules in particular - easy and fast.

https://www.npmjs.com/package/firebase-jest-testing/v/0.0.3-beta.1

  • It needs no client libraries, bypassing the compatibility issues between firebase-rules-testing and the 9.x client (and reducing install size by 100MB+)
  • It treats Firestore database as immutable, when Security Rules are tested

@samtstern
Copy link
Contributor

samtstern commented May 27, 2021

@Francesco-Lanciana or @inf3rnus does calling firestore.terminate() at the end of the test suite help with this?
https://firebase.google.com/docs/reference/js/firebase.firestore.Firestore#terminate

@looptheloop88
Copy link

Hi @samtstern, I tried adding firestore.terminate() to both the repro provided by @Francesco-Lanciana and @inf3rnus, but the warning is still there.

@akauppi
Copy link

akauppi commented May 31, 2021

I had a look at @Francesco-Lanciana 's sample, simplified it a bit, and eventually came to jestjs/jest#11464 (comment) which imho is closest to the root cause of this..


My experience differs from the OP in that --detectOpenHandles always mutes the warning and tests never hang. Not with @Francesco-Lanciana 's unchanged sample, either.

Other conclusions / tidbits:

  • the culprit cannot be @firebase/rules-unit-testing, since this occurs also with firebase-jest-testing that doesn't have it.
macOS 11.4
node 16.2.0
npm 7.13.0
firebase-tools 9.12.0

@samtstern
Copy link
Contributor

Thanks @akauppi and @Francesco-Lanciana for the information, seems like there's something else going on here!

@akauppi
Copy link

akauppi commented Jul 7, 2021

I managed to fix this, on my tool's side, today. Here's the code:

akauppi/firebase-jest-testing@05b1eb9#diff-ddeee476969de2a7660167c75333c7a9e882091000d49d4c3e3d566a83eb66b2R22-R74

I tap to the .onSnapshot calls that test code does, and Jest's afterAll. The API seen by test users is a subset of the normal Firebase Admin SDK API, but in addition listeners are automatically closed when Jest exists. This allows control to proceed to the OS level, which it doesn't without the release of those listeners.

@AndrewSouthpaw
Copy link

Has there been any movement around this? I also have the issue, and don't use @firebase/rules-unit-testing; only firebase-admin is involved. As soon as I await a call to Firestore, the test will end with that warning (even when the network call returns as expected). I've tried deleting all the apps via

afterAll(async () => {
  await Promise.all(admin.apps.map((app) => app?.delete()));
});

but that also doesn't work.

The --forceExit option only works if also run in conjunction with --runInBand, which causes a massive performance hit on test speeds.

Without fixing the issue, the test process hangs indefinitely in CI.

This is a pretty significant blocker, any chance this could be investigated more by the engineering team?

@samatcolumn
Copy link

I also ran into this issue however --forceExit seems to have worked for me. Still, it would be much better if there was a real solution since --forceExit can cover up some non-Firebase async issues that may arise in tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants