Improve performance of mock tests

Co-authored-by: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com>

app/main.ts

@@ -989,7 +989,7 @@ async function createWindow() {
 
   await safeLoadURL(
     mainWindow,
-    process.env.TEST_ELECTRON_SCRIPT != null
+    getEnvironment() === Environment.Test
       ? await prepareFileUrl([__dirname, '../test/index.html'])
       : await prepareFileUrl([__dirname, '../background.html'])
   );

package.json

@@ -40,13 +40,13 @@
     "prepare-staging-build": "node scripts/prepare_staging_build.js",
     "prepare-windows-cert": "node scripts/prepare_windows_cert.js",
     "test": "yarn test-node && yarn test-electron && yarn test-lint-intl && yarn test-eslint",
-    "test-electron": "node ts/scripts/test-electron.js",
+    "test-electron": "cross-env IS_TESTS=1 node ts/scripts/test-electron.js",
-    "test-release": "node ts/scripts/test-release.js",
+    "test-release": "cross-env IS_TESTS=1 node ts/scripts/test-release.js",
-    "test-node": "cross-env LANG=en-us electron-mocha --timeout 10000 --file test/setup-test-node.js --recursive test/modules ts/test-node ts/test-both",
+    "test-node": "cross-env IS_TESTS=1 LANG=en-us electron-mocha --timeout 10000 --file test/setup-test-node.js --recursive test/modules ts/test-node ts/test-both",
-    "test-mock": "cross-env NODE_ENV=test mocha --require ts/test-mock/setup-ci.js ts/test-mock/**/*_test.js",
+    "test-mock": "cross-env IS_TESTS=1 mocha --require ts/test-mock/setup-ci.js ts/test-mock/**/*_test.js",
-    "test-eslint": "mocha .eslint/rules/**/*.test.js --ignore-leaks",
+    "test-eslint": "cross-env IS_TESTS=1 mocha .eslint/rules/**/*.test.js --ignore-leaks",
-    "test-node-coverage": "nyc --reporter=lcov --reporter=text mocha --recursive test/modules ts/test-node ts/test-both",
+    "test-node-coverage": "cross-env IS_TESTS=1 nyc --reporter=lcov --reporter=text mocha --recursive test/modules ts/test-node ts/test-both",
-    "test-lint-intl": "ts-node ./build/intl-linter/linter.ts --test",
+    "test-lint-intl": "cross-env IS_TESTS=1 ts-node ./build/intl-linter/linter.ts --test",
     "eslint": "eslint --cache . --cache-strategy content --max-warnings 0",
     "lint": "run-s --print-label lint-prettier lint-css check:types eslint",
     "lint-deps": "node ts/util/lint/linter.js",

ts/components/Intl.tsx

@@ -8,7 +8,6 @@ import type { FormatXMLElementFn } from 'intl-messageformat';
 import type { LocalizerType } from '../types/Util';
 import type { ReplacementValuesType } from '../types/I18N';
 import * as log from '../logging/log';
-import { strictAssert } from '../util/assert';
 
 export type FullJSXType =
   | FormatXMLElementFn<JSX.Element | string>
@@ -36,16 +35,6 @@ export function Intl({
     return null;
   }
 
-  strictAssert(
-    !localizer.isLegacyFormat(id),
-    `Legacy message format is no longer supported ${id}`
-  );
-
-  strictAssert(
-    !Array.isArray(components),
-    `components cannot be an array for ICU message ${id}`
-  );
-
   const intl = localizer.getIntl();
   return <>{intl.formatMessage({ id }, components, {})}</>;
 }

ts/scripts/test-electron.ts

@@ -29,11 +29,10 @@ function launchElectron(attempt: number): string {
       cwd: ROOT_DIR,
       env: {
         ...process.env,
-        NODE_ENV: 'test',
+        // Setting NODE_ENV to test triggers main.ts to load
-        // Setting TEST_ELECTRON_SCRIPT to test triggers main.ts to load
         // 'test/index.html' instead of 'background.html', which loads the tests
         // via `test.js`
-        TEST_ELECTRON_SCRIPT: 'on',
+        NODE_ENV: 'test',
         TEST_QUIT_ON_COMPLETE: 'on',
       },
       encoding: 'utf8',

ts/state/ducks/user.ts

@@ -129,7 +129,6 @@ export function getEmptyState(): UserStateType {
     i18n: Object.assign(intlNotSetup, {
       getLocale: intlNotSetup,
       getIntl: intlNotSetup,
-      isLegacyFormat: intlNotSetup,
       getLocaleMessages: intlNotSetup,
       getLocaleDirection: intlNotSetup,
       getHourCyclePreference: intlNotSetup,

ts/test-both/types/setupI18n_test.ts

@@ -14,13 +14,6 @@ describe('setupI18n', () => {
   });
 
   describe('i18n', () => {
-    it('throws an error for legacy strings', () => {
-      assert.throws(() => {
-        // eslint-disable-next-line local-rules/valid-i18n-keys
-        i18n('legacystring');
-      }, /Legacy message format is no longer supported/);
-    });
-
     it('throws an error for unknown string', () => {
       assert.throws(() => {
         // eslint-disable-next-line local-rules/valid-i18n-keys
@@ -76,15 +69,4 @@ describe('setupI18n', () => {
       );
     });
   });
-
-  describe('isLegacyFormat', () => {
-    it('returns false for new format', () => {
-      assert.isFalse(
-        i18n.isLegacyFormat(
-          'icu:AddUserToAnotherGroupModal__toast--adding-user-to-group'
-        )
-      );
-      assert.isTrue(i18n.isLegacyFormat('softwareAcknowledgments'));
-    });
-  });
 });

ts/types/Util.ts

@@ -24,7 +24,6 @@ export type ReplacementValuesType = {
 export type LocalizerType = {
   (key: string, values?: ReplacementValuesType): string;
   getIntl(): IntlShape;
-  isLegacyFormat(key: string): boolean;
   getLocale(): string;
   getLocaleMessages(): LocaleMessagesType;
   getLocaleDirection(): LocaleDirection;

ts/util/setupI18n.tsx

@@ -84,7 +84,13 @@ function normalizeSubstitutions(
     return;
   }
   const normalized: ReplacementValuesType = {};
-  for (const [key, value] of Object.entries(substitutions)) {
+  const keys = Object.keys(substitutions);
+  if (keys.length === 0) {
+    return;
+  }
+  for (let i = 0; i < keys.length; i += 1) {
+    const key = keys[i];
+    const value = substitutions[key];
     if (typeof value === 'string') {
       normalized[key] = bidiIsolate(value);
     } else {
@@ -108,26 +114,11 @@ export function setupI18n(
   const intl = createCachedIntl(locale, filterLegacyMessages(messages));
 
   const localizer: LocalizerType = (key, substitutions) => {
-    strictAssert(
-      !localizer.isLegacyFormat(key),
-      `i18n: Legacy message format is no longer supported "${key}"`
-    );
-
-    strictAssert(
-      !Array.isArray(substitutions),
-      `i18n: Substitutions must be an object for ICU message "${key}"`
-    );
-
     const result = intl.formatMessage(
       { id: key },
       normalizeSubstitutions(substitutions)
     );
 
-    strictAssert(
-      typeof result === 'string',
-      'i18n: Formatted translation result must be a string, must use <Intl/> component to render JSX'
-    );
-
     strictAssert(result !== key, `i18n: missing translation for "${key}"`);
 
     return result;
@@ -136,9 +127,6 @@ export function setupI18n(
   localizer.getIntl = () => {
     return intl;
   };
-  localizer.isLegacyFormat = (key: string) => {
-    return !key.startsWith('icu:');
-  };
   localizer.getLocale = () => locale;
   localizer.getLocaleMessages = () => messages;
   localizer.getLocaleDirection = () => {

ts/util/unicodeBidi.ts

@@ -1,8 +1,6 @@
 // Copyright 2024 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
-import { Environment, getEnvironment } from '../environment';
-
 /**
  * Left-to-Right Isolate
  * Sets direction to LTR and isolates the embedded content from the surrounding text
@@ -210,7 +208,7 @@ export function _bidiIsolate(text: string): string {
  * ```
  */
 export function bidiIsolate(text: string): string {
-  if (getEnvironment() === Environment.Test) {
+  if (process.env.IS_TESTS != null) {
     // Turn this off in tests to make it easier to compare strings
     return text;
   }