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

Transaction.create is missing from @firebase/testing #2633

Closed
stari4ek opened this issue Feb 14, 2020 · 6 comments
Closed

Transaction.create is missing from @firebase/testing #2633

stari4ek opened this issue Feb 14, 2020 · 6 comments
Labels
api: firestore testing-sdk testing with emulator

Comments

@stari4ek
Copy link

[REQUIRED] Describe your environment

  • Operating System version: Linux 7u 5.4.5-050405-generic #201912181630 SMP Wed Dec 18 16:33:40 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
  • Firebase SDK version:
"@firebase/testing": "^0.16.8",
"@google-cloud/firestore": "^3.4.1",
"firebase-admin": "^8.9.2",
"firebase-functions": "^3.3.0",
  • Firebase Product: @firebase/testing

[REQUIRED] Describe the problem

Steps to reproduce:

Use Transaction.create on testing env. Transaction.set works well.

TypeError: tr.create is not a function
    at Function.registerInstanceInfo (src/app/iid/manager.ts:139:22)
    at processTicksAndRejections (internal/process/task_queues.js:89:5)
    at async Promise.all (index 0)

Relevant Code:

Approx. code:

const firebaseApp = firebase.initializeAdminApp({projectId: TEST_PROJECT_ID});
firebaseApp.firestore().runTransaction(async (tr) => {
   const instanceDocRef: DocumentReference = ...;
   // FAILS:
   await tr.create(instanceDocRef, {a: 'a'});
   // WORKS:
   await tr.set(instanceDocRef, {a: 'a'});
});
@google-oss-bot
Copy link
Contributor

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@wu-hui
Copy link
Contributor

wu-hui commented Feb 14, 2020

Why do you need a create in tests while there are no create in actual Transaction?

set will take care of creating the doc if it does not exist.

@wu-hui wu-hui closed this as completed Feb 14, 2020
@stari4ek
Copy link
Author

Are we talking about different Transaction?:

export class Transaction {
   ...
   /**
     * Create the document referred to by the provided `DocumentReference`.
     * The operation will fail the transaction if a document exists at the
     * specified location.
     *
     * @param documentRef A reference to the document to be create.
     * @param data The object data to serialize as the document.
     * @return This `Transaction` instance. Used for chaining method calls.
     */
    create<T>(documentRef: DocumentReference<T>, data: T): Transaction;
}

Transaction.create is used by production code, so when it's running under firestore emulator with @firestore/testing used - I'd expect that all create/set/update/delete would work.

@wu-hui
Copy link
Contributor

wu-hui commented Feb 14, 2020

Yes, we are talking about different transactions.

The Transaction you paste is from the Node SDK. It's a server SDK. What @firebase/testing creates for you is a client side JS Firestore. They are different.

I see you were no happy with my reply earlier, I apologize for not explaining this with deserved details.

@wilhuff
Copy link
Contributor

wilhuff commented Feb 14, 2020

It's unfortunate in this context that @firebase/testing is actually returning an instance of the web SDK, not the server-side Node.js SDK. #2112 catalogues all the differences between them.

In some sense this is a duplicate of #1324. Rules testing is an interesting new motivating use case for this though so we'll bump our internal priority on this in the backlog. PRs welcome in the meantime :-).

This is b/129358230 internally.

@stari4ek
Copy link
Author

@wu-hui, got it. Thank you.

Honestly speaking it's not that obvious where @firebase/testing is belong (client/server side). I've found it on https://firebase.google.com/docs/rules/unit-tests#firestore, and first assumptions is that it's "good start for unit tests" (on the backend).

But there're a bunch of issues when @firebase/testing is used this way (there is another one: firebase/firebase-tools#1971).

Only some issue on the github hinted me that firebase emulator does work well with firebase-admin (so no need in @firebase/testing) when FIRESTORE_EMULATOR_HOST is set.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: firestore testing-sdk testing with emulator
Projects
None yet
Development

No branches or pull requests

5 participants