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.

Sign up for GitHub

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

fix(fcm): Exporting all types of Messages so they can be used by consumers #1147

Merged
merged 9 commits into from
Feb 4, 2021
Merged

fix(fcm): Exporting all types of Messages so they can be used by consumers #1147

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e6336e3
feat: Exporting all types of Messages so they can be used by consumers
BorntraegerMarc Jan 28, 2021
6a69e71
feat(exportMessageTypes): Testing TokenMessage
BorntraegerMarc Feb 3, 2021
8755878
feat(exportMessageTypes): Added tests for all Message types
BorntraegerMarc Feb 3, 2021
b471877
feat(exportMessageTypes): Fixed build
BorntraegerMarc Feb 3, 2021
5f4b793
Merge branch 'master' into patch-1
BorntraegerMarc Feb 4, 2021
df80c59
feat(exportMessageTypes): Better unit tests
BorntraegerMarc Feb 4, 2021
dce2e00
feat(exportMessageTypes): Deleted unneeded separate TS test
BorntraegerMarc Feb 4, 2021
fca55b7
feat(exportMessageTypes): Fixed build
BorntraegerMarc Feb 4, 2021
e99eee0
feat(exportMessageTypes): Fixed linting
BorntraegerMarc Feb 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/messaging/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ export namespace messaging {
fcmOptions?: FcmOptions;
}

interface TokenMessage extends BaseMessage {
export interface TokenMessage extends BaseMessage {
token: string;
}

interface TopicMessage extends BaseMessage {
export interface TopicMessage extends BaseMessage {
topic: string;
}

interface ConditionMessage extends BaseMessage {
export interface ConditionMessage extends BaseMessage {
condition: string;
}

Expand Down
5 changes: 3 additions & 2 deletions test/unit/messaging/messaging.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ chai.use(chaiAsPromised);
const expect = chai.expect;

import Message = messaging.Message;
import TokenMessage = messaging.TokenMessage;
import MessagingOptions = messaging.MessagingOptions;
import MessagingPayload = messaging.MessagingPayload;
import MessagingDevicesResponse = messaging.MessagingDevicesResponse;
Expand Down Expand Up @@ -887,7 +888,7 @@ describe('Messaging', () => {
expect(messages.length).to.equal(3);
expect(stub!.args[0][1]).to.be.undefined;
messages.forEach((message, idx) => {
expect((message as any).token).to.equal(tokens[idx]);
expect((message as TokenMessage).token).to.equal(tokens[idx]);
expect(message.android).to.be.undefined;
expect(message.apns).to.be.undefined;
expect(message.data).to.be.undefined;
Expand Down Expand Up @@ -917,7 +918,7 @@ describe('Messaging', () => {
expect(messages.length).to.equal(3);
expect(stub!.args[0][1]).to.be.undefined;
messages.forEach((message, idx) => {
expect((message as any).token).to.equal(tokens[idx]);
expect((message as TokenMessage).token).to.equal(tokens[idx]);
expect(message.android).to.deep.equal(multicast.android);
expect(message.apns).to.be.deep.equal(multicast.apns);
expect(message.data).to.be.deep.equal(multicast.data);
Expand Down
40 changes: 40 additions & 0 deletions test/unit/typescript.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*!
* @license
* Copyright 2017 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

import * as chai from 'chai';
import * as chaiAsPromised from 'chai-as-promised';

import { messaging } from '../../src/messaging/index';

chai.should();
chai.use(chaiAsPromised);

const expect = chai.expect;

describe('Typescript', () => {
// This test was added as part of https://github.com/firebase/firebase-admin-node/issues/1146
it('should export all types of Messages so it can be used by lib users', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try to incorporate this test logic to the existing messaging.spec file. No need to carve out a new file for this. Easiest would be to list it under the existing test suite for the send() method:

      it(`should be fulfilled with a message ID given a TokenMessage`, () => {
        const message: TokenMessage = {token: 'test'}
        mockedRequests.push(mockSendRequest());
        return messaging.send(message)
          .should.eventually.equal('projects/projec_id/messages/message_id');
      });

     // Add similar test cases for other message types.

Also note that when you add a new test file, you have to also list it in index.spec.ts. Otherwise mocha won't run it. But, let's just add this to the existing unit test file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or alternatively you can have one test case in the suite for sendAll():

    it('should be fulfilled when called with different message types', () => {
      const messageIds = [
        'projects/projec_id/messages/1',
        'projects/projec_id/messages/2',
        'projects/projec_id/messages/3',
      ];
      const tokenMessage: TokenMessage = {token: 'test'};
      const topicMessage: TopicMessage = {topic: 'test'};
      const conditionMessage: ConditionMessage = {condition: 'test'};
      const messages: Message[] = [tokenMessage, topicMessage, conditionMessage];
      mockedRequests.push(mockBatchRequest(messageIds));
      return messaging.sendAll(messages)
        .then((response: BatchResponse) => {
          expect(response.successCount).to.equal(3);
          expect(response.failureCount).to.equal(0);
          response.responses.forEach((resp, idx) => {
            expect(resp.success).to.be.true;
            expect(resp.messageId).to.equal(messageIds[idx]);
            expect(resp.error).to.be.undefined;
          });
        });
    });

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thx for the guidance!

Also note that when you add a new test file, you have to also list it in index.spec.ts. Otherwise mocha won't run it

FYI mocha did run the new test. I also verified when I added the new file 🙂

Screenshot 2021-02-04 at 08 43 04

Source: https://github.com/firebase/firebase-admin-node/runs/1820854315

const tokenMessage: messaging.TokenMessage = { token: '' };
const topicMessage: messaging.TopicMessage = { topic: '' };
const conditionMessage: messaging.ConditionMessage = { condition: '' };
const allMessages: messaging.Message[] = [tokenMessage, topicMessage, conditionMessage]

allMessages.forEach((m) => expect(m).to.not.be.undefined);
});
});