[@exp] rules unit testing

[laurentpayot]

Hi, so far using firebase@exp has been great (35% bundle size reduction 🎉) but it broke all my unit tests 😕

If I use the following db

import * as firebase from '@firebase/rules-unit-testing'

const app = firebase.initializeTestApp({…})
export const db = app.firestore()

in (for instance)

const batch = writeBatch(db)

I get

FirebaseError: Type does not match the expected instance. Did you pass a reference from a different Firestore SDK?

Should I open an issue in firebase-tools or is there a workaround?

[laurentpayot]

@akauppi except a couple of firebase events triggered cloud functions my app is totally client-side based. So, client-side, I have CRUD functions for my entities that use firestore primitives. These CRUD functions have the database dependency injected as a parameter for easier testing.
When running my Jest tests I’m passing to my CRUD functions different @firebase/rules-unit-testing databases initialized with their respective test user auth data.
This way, with the current non-alpha release, I could run lots of tests to ensure that an entity created by user A cannot be modified by user B and basic security stuff like that.

But now I cannot inject firestore test databases anymore as their implementation is totally different from the firebase@exp firestore implementation I use on the client side.

Is it a clearer description?

[akauppi]

What I think is going on here is a collision between two separate but similar libraries. Let’s see it based on your first post:

import * as firebase from '@firebase/rules-unit-testing'

const app = firebase.initializeTestApp({…})
export const db = app.firestore()

All of this is based on rules-unit-testing which resembles the 8.x Firebase API but may have a completely different implementation. We won’t know, as users. Anyhow, it doesn’t even know the @exp API’s exist.

const batch = writeBatch(db)

Where did you get the writeBatch from? Presumably, it’s @exp and you are feeding it an object from the rules-unit-testing universe.

Do you see where you go off the road? I don’t know why it used to work. Maybe the rules-unit-testing implementation is close enough with the 8.x API that this wasn’t breaking things. But - conceptually - I imagine also the earlier code was ”broken”.

[Feiyang1]

@firebase/rules-unit-testing depends on firebase@8, so the firestore instance you got is a v8 implementation, while writeBatch() expects an exp instance.

We have some planned changes for @firebase/rules-unit-testing to address some of its current shortcomings and make it work with the new SDK. It's still being worked on, so unfortunately you can't use the exp package with the existing @firebase/rules-unit-testing until that comes out.

@yuchenshi Anything you want to add?

[akauppi]

@Feiyang1 Would you have any estimate on the @firebase/rules-unit-testing changes? A week? I was just today doing work on that, and considering bypassing it with the Firestore REST API. That would make rules testing detached from client libraries.

[Feiyang1]

We don't promise specific timelines, but I will say it's on our immediate roadmap.

[laurentpayot]

It’s been a looong month developing without touching existing Firestore stuff because of the lack of unit tests. Any news to share before Google I/O 2021?

[akauppi]

It’s been a looong month developing without touching existing Firestore stuff because of the lack of unit tests. Any news to share before Google I/O 2021?

I am working on side support. Updating a library I use for testing so that it’s not depending on rules-unit-testing but has its own code. Will hopefully be available by Wed.

[yuchenshi]

Support for rules unit testing has been merged in #4837 and will be included in the next exp release. tl;dr you'll be able to specify a mock user directly through useFirestoreEmulator(db, host, port, {mockUserToken: {sub: "myUid", ...}}). You can still use some emulator utilities in @firebase/rules-unit-testing to update rules or clear data, but there is no need to initializeTestApp through another library so you won't get the version conflicts.

[yuchenshi]

Aug 2021 update: We've released @firebase/rules-unit-testing v2, which is a major revamp and works with v9 JS SDK (modular / compat).

See our quickstarts on using the new RUTv2 API (docs: guide | reference). The instructions below are still possible for power users who wants to keep dependencies to a minimum (but you're pretty much on your own if you choose this path).

---

Earlier response:

As promised, useFirestoreEmulator(db, host, port, {mockUserToken: {...}}) is now released. Try it out with the new Beta modular SDK and let us know what you think! useDatabaseEmulator also got a similar upgrade and storage is coming soon also released.

[laurentpayot]

@wceolin As mentioned above, the only solution I found to calm TypeScript down was to do a type assertion like so:

import type { Firestore } from '@firebase/firestore'

const aliceDb = <Firestore><unknown>testEnv.authenticatedContext('alice').firestore()

[yuchenshi]

@laurentpayot @wceolin We've located the issue and is working on a fix now (#5500). Thanks a lot for your help!

Technical details on this: Firebase JS SDK v9 has a compatibility layer that allows v8-like non-modular APIs. The rules unit testing library is supposed to work with either that or the modular v9 APIs but due to a typing bug earlier TypeScript complains for the latter. #5500 fixes that, and type assertions are valid workarounds until the fix is released soon.

[laurentpayot]

The #5550 "solution" does calm TypeScript in the editor, but when actually running my tests with the test db injected in my front end code I get the same type errors as mentioned above 😐
So I have to keep type assertions for now…

[yuchenshi]

@laurentpayot See #5550 (comment). BTW, I'd really appreciate it if we can continue any further discussion on relevant issues instead of this thread.

[laurentpayot]

@yuchenshi the link I was reffering to in my previous comment is the one you want me to see.
Sure, I think the object of this discussion is now discussed enough. Thanks for all your time. 👍

[akauppi]

@yuchenshi Nice that Rules v2 is getting attention.

You may want to consider two minor things about the tests you linked to, before releasing:

1. https://github.com/firebase/quickstart-testing/pull/209/files#diff-6f539bff6bb3d5eeb2d2074b7d97c417466274d42384b3b8c504f94633ea3393R64-R68

   ..contains setup within tests themselves, which is not giving a good example. Rather, move those to beforeEach?

2. Instead of plain db for the database handle that acts with Alice's rights, you could name it dbAlice.

Why do these matter? For one, any sample you ship is educational to the larger audience, some of whom are novices and will simply copy-paste what's out there.

Second, tests should be even clearer and more readable than code, since they can be used also by managers and such to discuss about features - and see what's being tested.

Great that Firebase has good testing tools.

[rachelmyers]

@akauppi, thanks for spending so much time on this; it's clear you're invested and want great code examples. The PR will be updated before it's merged, but we can't promise we will or won't make those changes.

1. Setup within the test vs in a beforeEach depends on the test file. If each test setup is pretty unique, I have no problem putting the setup in each test. We'll have to see what the final state of the file is.
2. We use both db and making the name specific to a user in our samples. It depends on the other tests in the file. Again, it depends on the final state of this test suite.