stub storage differently so tests work in Firefox and IE (#1940)

* stub storage differently so tests work in Firefox and IE

* [AUTOMATED]: Prettier Code Styling

* rewrite test setup

* [AUTOMATED]: Prettier Code Styling

Author: Feiyang1

packages/performance/src/services/remote_config_service.test.ts

@@ -14,11 +14,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { stub, SinonStub, useFakeTimers, SinonFakeTimers } from 'sinon';
+import { stub, useFakeTimers, SinonFakeTimers, SinonStub } from 'sinon';
 import { expect } from 'chai';
 import { SettingsService } from './settings_service';
 import { CONFIG_EXPIRY_LOCAL_STORAGE_KEY } from '../constants';
-import { setupApi } from './api_service';
+import { setupApi, Api } from './api_service';
 import * as iidService from './iid_service';
 import { getConfig } from './remote_config_service';
 import { FirebaseApp } from '@firebase/app-types';
@@ -41,11 +41,10 @@ describe('Performance Monitoring > remote_config_service', () => {
   const APP_ID = '1:23r:web:fewq';
   const API_KEY = 'asdfghjk';
 
-  let fetchStub: SinonStub<[RequestInfo, RequestInit?], Promise<Response>>;
-  let storageGetItemStub: SinonStub<[string], string | null>;
   let clock: SinonFakeTimers;
 
   setupApi(self);
+  const ApiInstance = Api.getInstance();
 
   function storageGetItemFakeFactory(
     expiry: string,
@@ -67,17 +66,47 @@ describe('Performance Monitoring > remote_config_service', () => {
     settingsService.tracesSamplingRate = 1;
   }
 
-  beforeEach(() => {
-    fetchStub = stub(self, 'fetch');
-    storageGetItemStub = stub(self.localStorage, 'getItem');
+  // parameterized beforeEach. Should be called at beginning of each test.
+  function setup(
+    storageConfig: { expiry: string; config: string },
+    fetchConfig?: { reject: boolean; value?: Response }
+  ): {
+    storageGetItemStub: SinonStub<[string], string | null>;
+    fetchStub: SinonStub<[RequestInfo, RequestInit?], Promise<Response>>;
+  } {
+    const fetchStub = stub(self, 'fetch');
+
+    if (fetchConfig) {
+      fetchConfig.reject
+        ? fetchStub.rejects()
+        : fetchStub.resolves(fetchConfig.value);
+    }
+
     stub(iidService, 'getAuthTokenPromise').returns(
       Promise.resolve(AUTH_TOKEN)
     );
+
     clock = useFakeTimers(GLOBAL_CLOCK_NOW);
     SettingsService.prototype.firebaseAppInstance = ({
       options: { projectId: PROJECT_ID, appId: APP_ID, apiKey: API_KEY }
     } as unknown) as FirebaseApp;
-  });
+
+    // we need to stub the entire localStorage, because storage can't be stubbed in Firefox and IE.
+    // stubbing on self(window) seems to only work the first time (at least in Firefox), the subsequent
+    // tests will have the same stub. stub.reset() in afterEach doesn't help either. As a result, we stub on ApiInstance.
+    // https://github.com/sinonjs/sinon/issues/662
+    const storageStub = stub(ApiInstance, 'localStorage');
+    const getItemStub: SinonStub<[string], string | null> = stub();
+
+    storageStub.value({
+      getItem: getItemStub.callsFake(
+        storageGetItemFakeFactory(storageConfig.expiry, storageConfig.config)
+      ),
+      setItem: () => {}
+    });
+
+    return { storageGetItemStub: getItemStub, fetchStub };
+  }
 
   afterEach(() => {
     resetSettingsService();
@@ -88,15 +117,14 @@ describe('Performance Monitoring > remote_config_service', () => {
     it('gets the config from the local storage if available and valid', async () => {
       // After global clock. Config not expired.
       const EXPIRY_LOCAL_STORAGE_VALUE = '1556524895330';
-      storageGetItemStub.callsFake(
-        storageGetItemFakeFactory(
-          EXPIRY_LOCAL_STORAGE_VALUE,
-          STRINGIFIED_CONFIG
-        )
-      );
+      const { storageGetItemStub: getItemStub } = setup({
+        expiry: EXPIRY_LOCAL_STORAGE_VALUE,
+        config: STRINGIFIED_CONFIG
+      });
+
       await getConfig(IID);
 
-      expect(storageGetItemStub).to.be.called;
+      expect(getItemStub).to.be.called;
       expect(SettingsService.getInstance().loggingEnabled).to.be.true;
       expect(SettingsService.getInstance().logEndPointUrl).to.equal(LOG_URL);
       expect(SettingsService.getInstance().logSource).to.equal(LOG_SOURCE);
@@ -111,12 +139,12 @@ describe('Performance Monitoring > remote_config_service', () => {
     it('does not call remote config if a valid config is in local storage', async () => {
       // After global clock. Config not expired.
       const EXPIRY_LOCAL_STORAGE_VALUE = '1556524895330';
-      storageGetItemStub.callsFake(
-        storageGetItemFakeFactory(
-          EXPIRY_LOCAL_STORAGE_VALUE,
-          STRINGIFIED_CONFIG
-        )
-      );
+
+      const { fetchStub } = setup({
+        expiry: EXPIRY_LOCAL_STORAGE_VALUE,
+        config: STRINGIFIED_CONFIG
+      });
+
       await getConfig(IID);
 
       expect(fetchStub).not.to.be.called;
@@ -125,16 +153,15 @@ describe('Performance Monitoring > remote_config_service', () => {
     it('gets the config from RC if local version is not valid', async () => {
       // Expired local config.
       const EXPIRY_LOCAL_STORAGE_VALUE = '1556524895320';
-      storageGetItemStub.callsFake(
-        storageGetItemFakeFactory(
-          EXPIRY_LOCAL_STORAGE_VALUE,
-          'not a valid config and should not be used'
-        )
+
+      const { storageGetItemStub: getItemStub } = setup(
+        { expiry: EXPIRY_LOCAL_STORAGE_VALUE, config: STRINGIFIED_CONFIG },
+        { reject: false, value: new Response(STRINGIFIED_CONFIG) }
       );
-      fetchStub.resolves(new Response(STRINGIFIED_CONFIG));
+
       await getConfig(IID);
 
-      expect(storageGetItemStub).to.be.calledOnce;
+      expect(getItemStub).to.be.calledOnce;
       expect(SettingsService.getInstance().loggingEnabled).to.be.true;
       expect(SettingsService.getInstance().logEndPointUrl).to.equal(LOG_URL);
       expect(SettingsService.getInstance().logSource).to.equal(LOG_SOURCE);
@@ -149,13 +176,15 @@ describe('Performance Monitoring > remote_config_service', () => {
     it('does not change the default config if call to RC fails', async () => {
       // Expired local config.
       const EXPIRY_LOCAL_STORAGE_VALUE = '1556524895320';
-      storageGetItemStub.callsFake(
-        storageGetItemFakeFactory(
-          EXPIRY_LOCAL_STORAGE_VALUE,
-          'not a valid config and should not be used'
-        )
+
+      setup(
+        {
+          expiry: EXPIRY_LOCAL_STORAGE_VALUE,
+          config: 'not a valid config and should not be used'
+        },
+        { reject: true }
       );
-      fetchStub.rejects();
+
       await getConfig(IID);
 
       expect(SettingsService.getInstance().loggingEnabled).to.equal(false);
@@ -164,17 +193,19 @@ describe('Performance Monitoring > remote_config_service', () => {
     it('uses secondary configs if the response does not have all the fields', async () => {
       // Expired local config.
       const EXPIRY_LOCAL_STORAGE_VALUE = '1556524895320';
-      storageGetItemStub.callsFake(
-        storageGetItemFakeFactory(
-          EXPIRY_LOCAL_STORAGE_VALUE,
-          'not a valid config and should not be used'
-        )
-      );
       const STRINGIFIED_PARTIAL_CONFIG = `{"entries":{\
       "fpr_vc_network_request_sampling_rate":"0.250000",\
       "fpr_vc_session_sampling_rate":"0.250000","fpr_vc_trace_sampling_rate":"0.500000"},\
       "state":"UPDATE"}`;
-      fetchStub.resolves(new Response(STRINGIFIED_PARTIAL_CONFIG));
+
+      setup(
+        {
+          expiry: EXPIRY_LOCAL_STORAGE_VALUE,
+          config: 'not a valid config and should not be used'
+        },
+        { reject: false, value: new Response(STRINGIFIED_PARTIAL_CONFIG) }
+      );
+
       await getConfig(IID);
 
       expect(SettingsService.getInstance().loggingEnabled).to.be.true;
@@ -183,14 +214,15 @@ describe('Performance Monitoring > remote_config_service', () => {
     it('uses secondary configs if the response does not have any fields', async () => {
       // Expired local config.
       const EXPIRY_LOCAL_STORAGE_VALUE = '1556524895320';
-      storageGetItemStub.callsFake(
-        storageGetItemFakeFactory(
-          EXPIRY_LOCAL_STORAGE_VALUE,
-          'not a valid config and should not be used'
-        )
-      );
       const STRINGIFIED_PARTIAL_CONFIG = '{"state":"NO TEMPLATE"}';
-      fetchStub.resolves(new Response(STRINGIFIED_PARTIAL_CONFIG));
+
+      setup(
+        {
+          expiry: EXPIRY_LOCAL_STORAGE_VALUE,
+          config: 'not a valid config and should not be used'
+        },
+        { reject: false, value: new Response(STRINGIFIED_PARTIAL_CONFIG) }
+      );
       await getConfig(IID);
 
       expect(SettingsService.getInstance().loggingEnabled).to.be.true;