Skip to content

Commit

Permalink
Improve performance of mock tests
Browse files Browse the repository at this point in the history
Co-authored-by: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com>
  • Loading branch information
automated-signal and jamiebuilds-signal committed Feb 29, 2024
1 parent 7844e43 commit 5579c4f
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 64 deletions.
2 changes: 1 addition & 1 deletion app/main.ts
Expand Up @@ -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'])
);
Expand Down
14 changes: 7 additions & 7 deletions package.json
Expand Up @@ -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-release": "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-mock": "cross-env NODE_ENV=test mocha --require ts/test-mock/setup-ci.js ts/test-mock/**/*_test.js",
"test-eslint": "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-lint-intl": "ts-node ./build/intl-linter/linter.ts --test",
"test-electron": "cross-env IS_TESTS=1 node ts/scripts/test-electron.js",
"test-release": "cross-env IS_TESTS=1 node ts/scripts/test-release.js",
"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 IS_TESTS=1 mocha --require ts/test-mock/setup-ci.js ts/test-mock/**/*_test.js",
"test-eslint": "cross-env IS_TESTS=1 mocha .eslint/rules/**/*.test.js --ignore-leaks",
"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": "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",
Expand Down
11 changes: 0 additions & 11 deletions ts/components/Intl.tsx
Expand Up @@ -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>
Expand Down Expand Up @@ -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, {})}</>;
}
5 changes: 2 additions & 3 deletions ts/scripts/test-electron.ts
Expand Up @@ -29,11 +29,10 @@ function launchElectron(attempt: number): string {
cwd: ROOT_DIR,
env: {
...process.env,
NODE_ENV: 'test',
// Setting TEST_ELECTRON_SCRIPT to test triggers main.ts to load
// Setting NODE_ENV 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',
Expand Down
1 change: 0 additions & 1 deletion ts/state/ducks/user.ts
Expand Up @@ -129,7 +129,6 @@ export function getEmptyState(): UserStateType {
i18n: Object.assign(intlNotSetup, {
getLocale: intlNotSetup,
getIntl: intlNotSetup,
isLegacyFormat: intlNotSetup,
getLocaleMessages: intlNotSetup,
getLocaleDirection: intlNotSetup,
getHourCyclePreference: intlNotSetup,
Expand Down
18 changes: 0 additions & 18 deletions ts/test-both/types/setupI18n_test.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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'));
});
});
});
1 change: 0 additions & 1 deletion ts/types/Util.ts
Expand Up @@ -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;
Expand Down
26 changes: 7 additions & 19 deletions ts/util/setupI18n.tsx
Expand Up @@ -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 {
Expand All @@ -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;
Expand All @@ -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 = () => {
Expand Down
4 changes: 1 addition & 3 deletions 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
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 5579c4f

Please sign in to comment.