Functions-samples: Updating to latest Firebase SDK to causes tests to fail in quickstart/uppercase

[jedfonner]

Steps to reproduce

1. Upgrade libraries to the latest Firebase SDK in /quickstarts/uppercase/functions/package.json

...
  "dependencies": {
    "firebase-admin": "5.4.3",
    "firebase-functions": "0.7.1"
  },
...

1. Run npm install
2. Run npm run test

Expected behavior

Tests should pass

Actual behavior

The tests fail with the following error message. As far as I can tell, this was introduced by the 5.5.0 version of firebase-admin. The tests do not fail on version 5.4.3.

The devDependency libraries are also really out-of-date, but updating them to the latest chai, mocha and sinon does not fix the problem.

```

mocha --reporter spec

Cloud Functions
makeUpperCase
Uppercasing undefined null
1) should upper case input and write it to /uppercase
addMessage
2) should return a 303 redirect

0 passing (312ms)
2 failing

1) Cloud Functions makeUpperCase should upper case input and write it to /uppercase:
TypeError: Cannot read property 'toUpperCase' of null
at exports.makeUppercase.functions.database.ref.onWrite.event (index.js:57:33)
at Object. (node_modules/firebase-functions/lib/cloud-functions.js:59:27)
at Generator.next ()
at node_modules/firebase-functions/lib/cloud-functions.js:28:71
at __awaiter (node_modules/firebase-functions/lib/cloud-functions.js:24:12)
at Object.cloudFunction [as makeUppercase] (node_modules/firebase-functions/lib/cloud-functions.js:53:36)
at Context.it (test/test.js:112:50)

2) Cloud Functions addMessage should return a 303 redirect:
TypeError: Attempted to wrap undefined property database as function
at checkWrappedMethod (node_modules/sinon/lib/sinon/util/core.js:78:29)
at Object.wrapMethod (node_modules/sinon/lib/sinon/util/core.js:129:21)
at Object.stub (node_modules/sinon/lib/sinon/stub.js:67:26)
at Context.it (test/test.js:130:28)

```

[cinmay]

I had the same problem. In the makeUpperCase test. The DeltaSnapshot interface has changed with the new update. It now requires an extra parameter 'instance'. To make the test pass change line 84 from
data: new functions.database.DeltaSnapshot(null, null, null, 'input'),
to
data: new functions.database.DeltaSnapshot(null, null, null, null, 'input'),

I have not looked into why addMessage fails.

[nicolasgarnier]

@laurenzlong since you wrote these tests, could you have a look into issue 2) if you have time? I had a quick look and saw nothing obvious that would explain this.

[nicolasgarnier]

Thanks @cinmay I'll send a PR for this with your update to fix issue 1)

[nicolasgarnier]

Was just thinking of issue 2):

It may happen if admin.database is now dynamically added by the admin.config() call. Since we are mocking it, then admin.database is not added and stays undefined.

So maybe adding admin.database = () => null; at Line 56 would do the trick?

[laurenzlong]

Sorry it was a bug in the last SDK version, just released 0.7.4, should work now. (I think you'd have to revert your last PR)

[nicolasgarnier]

😭 😭

[nicolasgarnier]

OK reverted! Also changed the dependency to 0/7/4.

I'll close this but feel free to re-open if there are still issues with the tests.

[jedfonner]

@nicolasgarnier I think this issue should be reopened. The update to 0.7.4 fixed one of the tests but not the other. The addMessage test is still failing with same error as before: Attempted to wrap undefined property database as function. FYI @laurenzlong

Here's the output of running npm run test in quickstarts/uppercase/functions using latest master:

> mocha --reporter spec

  Cloud Functions
    makeUpperCase
Uppercasing undefined input
      ✓ should upper case input and write it to /uppercase
    addMessage
      1) should return a 303 redirect

  1 passing (353ms)
  1 failing

  1) Cloud Functions addMessage should return a 303 redirect:
     TypeError: Attempted to wrap undefined property database as function
      at checkWrappedMethod (node_modules/sinon/lib/sinon/util/core.js:78:29)
      at Object.wrapMethod (node_modules/sinon/lib/sinon/util/core.js:129:21)
      at Object.stub (node_modules/sinon/lib/sinon/stub.js:67:26)
      at Context.it (test/test.js:130:28)

[nicolasgarnier]

Thanks @jedfonner

Can you try adding admin.database = () => null; at Line 56? :) Just in case that fixe it.

[jedfonner]

Assuming you meant line 56 of /quickstarts/uppercase/functions/test/test.js, I tried that and it had no effect.

[nicolasgarnier]

Looks like the database property can't be mocked by sinon v1. I'm guessing this is related to: https://github.com/sinonjs/sinon/issues/1018

We need to try to update to a newer version of sinon which is supposed to have that fixed since v2.

[nicolasgarnier]

Ok I was able to get it to work.

First you need the latest version of sinon:

npm install --save sinon@latest

Then change the way we stub admin.database because it is a getter and not a property:

Change test.js L.131 from:

databaseStub.returns( { ref: refStub });

to:

databaseStub.get(() => (() => ({ ref: refStub })));

I'll try to get the docs updated tomorrow.

[laurenzlong]

Great job!! Do you mean updating this docs page? https://firebase.google.com/docs/functions/unit-testing

[nicolasgarnier]

Yes! (and the sample which the snippets are pulled from)

[laurenzlong]

Perfect!