Add Firefox testing to Auth (#6522)

dwyfrequency · web-flow · commit dc7574e0f2ee · 2022-08-15T16:03:35.000-04:00
* Update version test to accomidate multiple browsers

* Add notes and console.logs

* Update tests to accomidate firefox browser

* Remove console.log

* Update browsers based on environment variable

* Update browsers when unit test selected

* Update karma config to take env variable

* Add test github action

* Update github workflow

* Update build command

* Update format in test changed auth

* Remove stand alone action for firefox auth

* Add expect for spys, and test command for browser unit

* Update spy to use proper sinon chai syntax
diff --git a/.github/workflows/test-changed-auth.yml b/.github/workflows/test-changed-auth.yml
@@ -7,32 +7,66 @@ env:
   DETECT_CHROMEDRIVER_VERSION: true
 
 jobs:
-  test:
-    name: Test Auth If Changed
+  test-chrome:
+    name: Test Auth on Chrome and Node If Changed
     runs-on: ubuntu-latest
 
     steps:
-    # install Chrome first, so the correct version of webdriver can be installed by chromedriver when setting up the repo
-    - name: install Chrome stable
-      run: |
-        sudo apt-get update
-        sudo apt-get install google-chrome-stable
-    - name: Checkout Repo
-      uses: actions/checkout@master
-      with:
-        # This makes Actions fetch all Git history so run-changed script can diff properly.
-        fetch-depth: 0
-    - name: Set up Node (14)
-      uses: actions/setup-node@v2
-      with:
-        node-version: 14.x
-    - name: Bump Node memory limit
-      run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV
-    - name: Test setup and yarn install
-      run: |
-        cp config/ci.config.json config/project.json
-        yarn
-    - name: build
-      run: yarn build:changed auth
-    - name: Run tests on changed packages
-      run: xvfb-run yarn test:changed auth
+      # install Chrome first, so the correct version of webdriver can be installed by chromedriver when setting up the repo
+      - name: install Chrome stable
+        run: |
+          sudo apt-get update
+          sudo apt-get install google-chrome-stable
+      - name: Checkout Repo
+        uses: actions/checkout@master
+        with:
+          # This makes Actions fetch all Git history so run-changed script can diff properly.
+          fetch-depth: 0
+      - name: Set up Node (14)
+        uses: actions/setup-node@v2
+        with:
+          node-version: 14.x
+      - name: Bump Node memory limit
+        run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV
+      - name: Test setup and yarn install
+        run: |
+          cp config/ci.config.json config/project.json
+          yarn
+      - name: build
+        run: yarn build:changed auth
+      - name: Run tests on changed packages
+        run: xvfb-run yarn test:changed auth
+  test-firefox:
+    name: Test Auth on Firefox If Changed
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: install Firefox stable
+        run: |
+          sudo apt-get update
+          sudo apt-get install firefox
+      - name: Checkout Repo
+        uses: actions/checkout@master
+        with:
+          # This makes Actions fetch all Git history so run-changed script can diff properly.
+          fetch-depth: 0
+      - name: Set up Node (14)
+        uses: actions/setup-node@v2
+        with:
+          node-version: 14.x
+      - name: Bump Node memory limit
+        run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV
+      - name: Test setup and yarn install
+        run: |
+          cp config/ci.config.json config/project.json
+          yarn
+      - name: build
+        run: yarn build:changed auth
+      - name: Run tests on auth changed packages
+        run: xvfb-run yarn --cwd packages/auth test:browser:unit
+        env:
+          BROWSERS: 'Firefox'
+      - name: Run tests on auth-compat changed packages
+        run: xvfb-run yarn --cwd packages/auth-compat test:browser:unit
+        env:
+          BROWSERS: 'Firefox'
diff --git a/packages/auth-compat/karma.conf.js b/packages/auth-compat/karma.conf.js
@@ -22,6 +22,7 @@ const files = ['src/**/*.test.ts'];
 
 module.exports = function (config) {
   const karmaConfig = Object.assign({}, karmaBase, {
+    browsers: getTestBrowsers(argv),
     // files to load into karma
     files: getTestFiles(),
     preprocessors: { '**/*.ts': ['webpack', 'sourcemap'] },
@@ -43,6 +44,14 @@ function getTestFiles() {
   }
 }
 
+function getTestBrowsers(argv) {
+  let browsers = ['ChromeHeadless'];
+  if (process.env?.BROWSERS && argv.unit) {
+    browsers = process.env?.BROWSERS?.split(',');
+  }
+  return browsers;
+}
+
 function getClientConfig() {
   if (!argv.integration) {
     return {};
diff --git a/packages/auth-compat/package.json b/packages/auth-compat/package.json
@@ -34,6 +34,7 @@
     "test:all": "run-p test:browser test:node test:integration",
     "test:ci": "node ../../scripts/run_tests_in_ci.js -s test:all",
     "test:browser": "karma start --single-run",
+    "test:browser:unit": "karma start --single-run --unit",
     "test:browser:integration": "karma start --single-run --integration",
     "test:node": "ts-node -O '{\"module\": \"commonjs\", \"target\": \"es6\"}' scripts/run_node_tests.ts",
     "test:node:integration": "ts-node -O '{\"module\": \"commonjs\", \"target\": \"es6\"}' scripts/run_node_tests.ts --integration",
diff --git a/packages/auth/karma.conf.js b/packages/auth/karma.conf.js
@@ -20,6 +20,7 @@ const { argv } = require('yargs');
 
 module.exports = function (config) {
   const karmaConfig = Object.assign({}, karmaBase, {
+    browsers: getTestBrowsers(argv),
     // files to load into karma
     files: getTestFiles(argv),
     // frameworks to use
@@ -52,6 +53,14 @@ function getTestFiles(argv) {
   }
 }
 
+function getTestBrowsers(argv) {
+  let browsers = ["ChromeHeadless"]; 
+  if (process.env?.BROWSERS && argv.unit) {
+    browsers = process.env?.BROWSERS?.split(',');
+  } 
+  return browsers;
+}
+
 function getClientConfig(argv) {
   if (!argv.local) {
     return {};
diff --git a/packages/auth/src/core/util/version.test.ts b/packages/auth/src/core/util/version.test.ts
@@ -18,7 +18,8 @@
 import { SDK_VERSION } from '@firebase/app';
 import { expect } from 'chai';
 import { ClientPlatform, _getClientVersion } from './version';
-import { isNode } from '@firebase/util';
+import { getUA, isNode } from '@firebase/util';
+import { _getBrowserName } from './browser';
 
 describe('core/util/_getClientVersion', () => {
   if (isNode()) {
@@ -33,15 +34,15 @@ describe('core/util/_getClientVersion', () => {
     context('browser', () => {
       it('should set the correct version', () => {
         expect(_getClientVersion(ClientPlatform.BROWSER)).to.eq(
-          `Chrome/JsCore/${SDK_VERSION}/FirebaseCore-web`
+          `${_getBrowserName(getUA())}/JsCore/${SDK_VERSION}/FirebaseCore-web`
         );
       });
     });
 
     context('worker', () => {
       it('should set the correct version', () => {
         expect(_getClientVersion(ClientPlatform.WORKER)).to.eq(
-          `Chrome-Worker/JsCore/${SDK_VERSION}/FirebaseCore-web`
+          `${_getBrowserName(getUA())}-Worker/JsCore/${SDK_VERSION}/FirebaseCore-web`
         );
       });
     });
diff --git a/packages/auth/src/platform_browser/persistence/local_storage.test.ts b/packages/auth/src/platform_browser/persistence/local_storage.test.ts
@@ -65,13 +65,15 @@ describe('platform_browser/persistence/local_storage', () => {
   });
 
   describe('#isAvailable', () => {
+    afterEach(() => sinon.restore());
+
     it('should emit false if localStorage setItem throws', async () => {
-      sinon.stub(localStorage, 'setItem').throws(new Error('nope'));
+      sinon.stub(Storage.prototype, 'setItem').throws(new Error('nope'));
       expect(await persistence._isAvailable()).to.be.false;
     });
 
     it('should emit false if localStorage removeItem throws', async () => {
-      sinon.stub(localStorage, 'removeItem').throws(new Error('nope'));
+      sinon.stub(Storage.prototype, 'removeItem').throws(new Error('nope'));
       expect(await persistence._isAvailable()).to.be.false;
     });
 
diff --git a/packages/auth/src/platform_browser/persistence/session_storage.test.ts b/packages/auth/src/platform_browser/persistence/session_storage.test.ts
@@ -63,13 +63,15 @@ describe('platform_browser/persistence/session_storage', () => {
     });
 
     describe('#isAvailable', () => {
+      afterEach(() => sinon.restore());
+
       it('should emit false if sessionStorage setItem throws', async () => {
-        sinon.stub(sessionStorage, 'setItem').throws(new Error('nope'));
-        expect(await persistence._isAvailable()).to.be.false;
+      sinon.stub(Storage.prototype, 'setItem').throws(new Error('nope'));
+      expect(await persistence._isAvailable()).to.be.false;
       });
 
       it('should emit false if sessionStorage removeItem throws', async () => {
-        sinon.stub(sessionStorage, 'removeItem').throws(new Error('nope'));
+        sinon.stub(Storage.prototype, 'removeItem').throws(new Error('nope'));
         expect(await persistence._isAvailable()).to.be.false;
       });
 
diff --git a/packages/auth/src/platform_cordova/popup_redirect/events.test.ts b/packages/auth/src/platform_cordova/popup_redirect/events.test.ts
@@ -36,11 +36,8 @@ use(sinonChai);
 
 describe('platform_cordova/popup_redirect/events', () => {
   let auth: TestAuth;
-  let storageStub: sinon.SinonStubbedInstance<typeof localStorage>;
-
   beforeEach(async () => {
     auth = await testAuth();
-    storageStub = sinon.stub(localStorage);
   });
 
   afterEach(() => {
@@ -73,28 +70,28 @@ describe('platform_cordova/popup_redirect/events', () => {
 
   describe('_savePartialEvent', () => {
     it('sets the event', async () => {
+      const spy = sinon.spy(Storage.prototype, 'setItem');
       const event = _generateNewEvent(auth, AuthEventType.REAUTH_VIA_REDIRECT);
       await _savePartialEvent(auth, event);
-      expect(storageStub.setItem).to.have.been.calledWith(
-        'firebase:authEvent:test-api-key:test-app',
-        JSON.stringify(event)
-      );
+      expect(spy).to.have.been.calledWith('firebase:authEvent:test-api-key:test-app',
+      JSON.stringify(event));
     });
   });
 
   describe('_getAndRemoveEvent', () => {
     it('returns null if no event is present', async () => {
-      storageStub.getItem.returns(null);
+      sinon.stub(Storage.prototype, 'getItem').returns(null);
       expect(await _getAndRemoveEvent(auth)).to.be.null;
     });
 
     it('returns the event and deletes the key if present', async () => {
       const event = JSON.stringify(
         _generateNewEvent(auth, AuthEventType.REAUTH_VIA_REDIRECT)
       );
-      storageStub.getItem.returns(event);
+      sinon.stub(Storage.prototype, 'getItem').returns(event);
+      const spy = sinon.spy(Storage.prototype, 'removeItem');
       expect(await _getAndRemoveEvent(auth)).to.eql(JSON.parse(event));
-      expect(storageStub.removeItem).to.have.been.calledWith(
+      expect(spy).to.have.been.calledWith(
         'firebase:authEvent:test-api-key:test-app'
       );
     });
