Enable strict mode and remove module prop hacks.

yuchenshi · yuchenshi · commit f7962efcaeb4 · 2021-03-12T13:00:59.000-08:00
diff --git a/packages-exp/auth-exp/test/integration/webdriver/static/index.html b/packages-exp/auth-exp/test/integration/webdriver/static/index.html
@@ -1,2 +1,6 @@
 <!DOCTYPE html>
-<script src="dist/bundle.js"></script>
+<!--
+  type=module makes browers treat it as an ES6 module and enforces strict mode. This helps catch
+  some errors in the test (e.g. extending frozen objects or accidentally creating global variables).
+-->
+<script type="module" src="dist/bundle.js"></script>
diff --git a/packages-exp/auth-exp/test/integration/webdriver/static/index.js b/packages-exp/auth-exp/test/integration/webdriver/static/index.js
@@ -24,18 +24,20 @@ import * as persistence from './persistence';
 import { initializeApp } from '@firebase/app-exp';
 import { getAuth, useAuthEmulator } from '@firebase/auth-exp';
 
-window.core = { ...core };
-window.anonymous = { ...anonymous };
-window.redirect = { ...redirect };
-window.popup = { ...popup };
-window.email = { ...email };
-window.persistence = { ...persistence };
+window.core = core;
+window.anonymous = anonymous;
+window.redirect = redirect;
+window.popup = popup;
+window.email = email;
+window.persistence = persistence;
+
+window.auth = null;
 
 // The config and emulator URL are injected by the test. The test framework
 // calls this function after that injection.
 window.startAuth = async () => {
   const app = initializeApp(firebaseConfig);
-  auth = getAuth(app);
+  const auth = getAuth(app);
   useAuthEmulator(auth, emulatorUrl);
   window.auth = auth;
 };
diff --git a/packages-exp/auth-exp/test/integration/webdriver/static/popup.js b/packages-exp/auth-exp/test/integration/webdriver/static/popup.js
@@ -30,46 +30,46 @@ import {
 // pass data back to the main Node process. Because of that setup, we can't
 // return the popup tasks as pending promises as they won't resolve until
 // the WebDriver is allowed to do other stuff. Instead, we'll store the
-// promises on the window and provide a way to retrieve them later, unblocking
+// promises in variables and provide a way to retrieve them later, unblocking
 // the WebDriver process.
+let popupPromise = null;
+let popupCred = null;
+let errorCred = null;
 
 export function idpPopup(optProvider) {
   const provider = optProvider
     ? new OAuthProvider(optProvider)
     : new GoogleAuthProvider();
-  window.popup.popupPromise = signInWithPopup(auth, provider);
+  popupPromise = signInWithPopup(auth, provider);
 }
 
 export function idpReauthPopup() {
-  window.popup.popupPromise = reauthenticateWithPopup(
+  popupPromise = reauthenticateWithPopup(
     auth.currentUser,
     new GoogleAuthProvider()
   );
 }
 
 export function idpLinkPopup() {
-  window.popup.popupPromise = linkWithPopup(
-    auth.currentUser,
-    new GoogleAuthProvider()
-  );
+  popupPromise = linkWithPopup(auth.currentUser, new GoogleAuthProvider());
 }
 
 export function popupResult() {
-  return window.popup.popupPromise;
+  return popupPromise;
 }
 
 export async function generateCredentialFromResult() {
-  const result = await window.popup.popupPromise;
-  window.popup.popupCred = GoogleAuthProvider.credentialFromResult(result);
-  return window.popup.popupCred;
+  const result = await popupPromise;
+  popupCred = GoogleAuthProvider.credentialFromResult(result);
+  return popupCred;
 }
 
 export async function signInWithPopupCredential() {
-  return signInWithCredential(auth, window.popup.popupCred);
+  return signInWithCredential(auth, popupCred);
 }
 
 export async function linkWithErrorCredential() {
-  await linkWithCredential(auth.currentUser, window.popup.errorCred);
+  await linkWithCredential(auth.currentUser, errorCred);
 }
 
 // These below are not technically popup functions but they're helpers for
@@ -93,7 +93,7 @@ export async function tryToSignInUnverified(email) {
       )
     );
   } catch (e) {
-    window.popup.errorCred = FacebookAuthProvider.credentialFromError(e);
+    errorCred = FacebookAuthProvider.credentialFromError(e);
     throw e;
   }
 }
diff --git a/packages-exp/auth-exp/test/integration/webdriver/static/redirect.js b/packages-exp/auth-exp/test/integration/webdriver/static/redirect.js
@@ -27,6 +27,9 @@ import {
   signInWithRedirect
 } from '@firebase/auth-exp';
 
+let redirectCred = null;
+let errorCred = null;
+
 export function idpRedirect(optProvider) {
   const provider = optProvider
     ? new OAuthProvider(optProvider)
@@ -48,18 +51,16 @@ export function redirectResult() {
 
 export async function generateCredentialFromRedirectResultAndStore() {
   const result = await getRedirectResult(auth);
-  window.redirect.redirectCred = GoogleAuthProvider.credentialFromResult(
-    result
-  );
-  return window.redirect.redirectCred;
+  redirectCred = GoogleAuthProvider.credentialFromResult(result);
+  return redirectCred;
 }
 
 export async function signInWithRedirectCredential() {
-  return signInWithCredential(auth, window.redirect.redirectCred);
+  return signInWithCredential(auth, redirectCred);
 }
 
 export async function linkWithErrorCredential() {
-  await linkWithCredential(auth.currentUser, window.redirect.errorCred);
+  await linkWithCredential(auth.currentUser, errorCred);
 }
 
 // These below are not technically redirect functions but they're helpers for
@@ -83,7 +84,7 @@ export async function tryToSignInUnverified(email) {
       )
     );
   } catch (e) {
-    window.redirect.errorCred = FacebookAuthProvider.credentialFromError(e);
+    errorCred = FacebookAuthProvider.credentialFromError(e);
     throw e;
   }
 }
diff --git a/packages-exp/auth-exp/test/integration/webdriver/util/auth_driver.ts b/packages-exp/auth-exp/test/integration/webdriver/util/auth_driver.ts
@@ -31,6 +31,15 @@ import { authTestServer } from './test_server';
 const START_FUNCTION = 'startAuth';
 const PASSED_ARGS = '...Array.prototype.slice.call(arguments, 0, -1)';
 
+type DriverCallResult =
+  | { type: 'success'; value: string /* JSON stringified */ }
+  | {
+      type: 'error';
+      fields: string /* JSON stringified */;
+      message: string;
+      stack: string;
+    };
+
 /** Helper wraper around the WebDriver object */
 export class AuthDriver {
   webDriver!: WebDriver;
@@ -52,30 +61,27 @@ export class AuthDriver {
     // serialization which blows up the whole thing. It's okay though; this is
     // an integration test: we don't care about the internal (hidden) values of
     // these objects.
-    const {
-      type,
-      value
-    }: {
-      type: string;
-      value: string;
-    } = await this.webDriver.executeAsyncScript(
+    const result = await this.webDriver.executeAsyncScript<DriverCallResult>(
       `
       var callback = arguments[arguments.length - 1];
       ${fn}(${PASSED_ARGS}).then(result => {
         callback({type: 'success', value: JSON.stringify(result)});
       }).catch(e => {
-        callback({type: 'error', value: JSON.stringify(e)});
+        callback({type: 'error', message: e.message, stack: e.stack, fields: JSON.stringify(e)});
       });
     `,
       ...args
     );
 
-    const parsed: object = JSON.parse(value);
-    if (type === 'success') {
-      return JSON.parse(value) as T;
+    if (result.type === 'success') {
+      return JSON.parse(result.value) as T;
     } else {
-      const e = new Error('Test promise rejection');
-      Object.assign(e, parsed);
+      const e = new Error(result.message);
+      const stack = e.stack;
+      Object.assign(e, JSON.parse(result.fields));
+
+      const trimmedDriverStack = result.stack.split('at eval (')[0];
+      e.stack = `${trimmedDriverStack}\nfrom WebDriver call ${fn}(...)\n${stack}`;
       throw e;
     }
   }
diff --git a/packages-exp/auth-exp/test/integration/webdriver/util/test_runner.ts b/packages-exp/auth-exp/test/integration/webdriver/util/test_runner.ts
@@ -60,9 +60,6 @@ setTimeout(() => {
       before(async function () {
         this.timeout(20000); // Starting browsers can be slow.
         await DRIVER.start(browser);
-
-        // Prime for the first test.
-        await DRIVER.reset();
       });
 
       after(async () => {
@@ -72,11 +69,8 @@ setTimeout(() => {
       // It's assumed that the tests will start with a clean slate (i.e.
       // no storage).
       beforeEach(async () => {
-        await DRIVER.injectConfigAndInitAuth();
-      });
-
-      afterEach(async () => {
         await DRIVER.reset();
+        await DRIVER.injectConfigAndInitAuth();
       });
 
       for (const { title, generator } of SUITES) {
