Skip to content

Commit fa0a7c0

Browse files
tom-andersentom-andersen
authored
Refactor Firestore tests to use targetBackend parameter (#6999)
* Refactor Firestore target backend parameters to simplify and better support preprod * Undo `only` in test * Formatting * Fix * Replace local with targetBackend=emulator * Fix * Run directly from ts file to avoid compiler problems. * Remove unused dependency * Remove unused dependency * Remove unused dependency * Fix test when running on emulator * Override linter * Add comments
1 parent 42fa2a1 commit fa0a7c0

File tree

12 files changed

+187
-130
lines changed

12 files changed

+187
-130
lines changed

‎.vscode/launch.json

+2-4
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,7 @@
7373
"--exit"
7474
],
7575
"env": {
76-
"FIRESTORE_EMULATOR_PORT" : "8080",
77-
"FIRESTORE_EMULATOR_PROJECT_ID" : "test-emulator"
76+
"FIRESTORE_TARGET_BACKEND" : "emulator",
7877
},
7978
"sourceMaps": true,
8079
"protocol": "inspector"
@@ -95,8 +94,7 @@
9594
],
9695
"env": {
9796
"USE_MOCK_PERSISTENCE": "YES",
98-
"FIRESTORE_EMULATOR_PORT" : "8080",
99-
"FIRESTORE_EMULATOR_PROJECT_ID" : "test-emulator"
97+
"FIRESTORE_TARGET_BACKEND" : "emulator",
10098
},
10199
"sourceMaps": true,
102100
"protocol": "inspector"

‎packages/firestore-compat/karma.conf.js

+16-24
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ const karmaBase = require('../../config/karma.base');
1919
const { argv } = require('yargs');
2020

2121
module.exports = function (config) {
22-
const karmaConfig = Object.assign({}, karmaBase, {
22+
const karmaConfig = {
23+
...karmaBase,
24+
2325
// files to load into karma
2426
files: getTestFiles(argv),
2527

@@ -29,12 +31,20 @@ module.exports = function (config) {
2931

3032
// frameworks to use
3133
// available frameworks: https://npmjs.org/browse/keyword/karma-adapter
32-
frameworks: ['mocha'],
34+
frameworks: ['mocha']
35+
};
3336

34-
client: Object.assign({}, karmaBase.client, {
35-
firestoreSettings: getFirestoreSettings(argv)
36-
})
37-
});
37+
if (argv.targetBackend) {
38+
karmaConfig.client = {
39+
...karmaConfig.client,
40+
targetBackend: argv.targetBackend
41+
};
42+
} else if (argv.local) {
43+
karmaConfig.client = {
44+
...karmaConfig.client,
45+
targetBackend: 'emulator'
46+
};
47+
}
3848

3949
config.set(karmaConfig);
4050
};
@@ -48,22 +58,4 @@ function getTestFiles(argv) {
4858
return [integrationTests];
4959
}
5060

51-
/**
52-
* If the --local argument is passed, returns a {host, ssl} FirestoreSettings
53-
* object that point to localhost instead of production.
54-
*/
55-
function getFirestoreSettings(argv) {
56-
if (argv.local) {
57-
return {
58-
host: 'localhost:8080',
59-
ssl: false
60-
};
61-
} else {
62-
return {
63-
host: 'firestore.googleapis.com',
64-
ssl: true
65-
};
66-
}
67-
}
68-
6961
module.exports.files = getTestFiles(argv);

‎packages/firestore-compat/test/util/settings.ts

+59-20
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
* limitations under the License.
1616
*/
1717

18-
import * as firestore from '@firebase/firestore-types';
19-
2018
/**
2119
* NOTE: These helpers are used by api/ tests and therefore may not have any
2220
* dependencies on src/ files.
@@ -25,38 +23,79 @@ import * as firestore from '@firebase/firestore-types';
2523
// eslint-disable-next-line @typescript-eslint/no-explicit-any
2624
declare const __karma__: any;
2725

26+
enum TargetBackend {
27+
EMULATOR = 'emulator',
28+
QA = 'qa',
29+
NIGHTLY = 'nightly',
30+
PROD = 'prod'
31+
}
32+
2833
// eslint-disable-next-line @typescript-eslint/no-require-imports
2934
const PROJECT_CONFIG = require('../../../../config/project.json');
3035

31-
const EMULATOR_PORT = process.env.FIRESTORE_EMULATOR_PORT;
32-
const EMULATOR_PROJECT_ID = process.env.FIRESTORE_EMULATOR_PROJECT_ID;
33-
export const USE_EMULATOR = !!EMULATOR_PORT;
36+
const TARGET_BACKEND: TargetBackend = getTargetBackend();
3437

35-
const EMULATOR_FIRESTORE_SETTING = {
36-
host: `localhost:${EMULATOR_PORT}`,
37-
ssl: false
38-
};
38+
export const USE_EMULATOR: boolean = TARGET_BACKEND === TargetBackend.EMULATOR;
3939

40-
const PROD_FIRESTORE_SETTING = {
41-
host: 'firestore.googleapis.com',
42-
ssl: true
40+
export const DEFAULT_SETTINGS = {
41+
host: getFirestoreHost(TARGET_BACKEND),
42+
ssl: getSslEnabled(TARGET_BACKEND)
4343
};
4444

45-
export const DEFAULT_SETTINGS = getDefaultSettings();
46-
4745
// eslint-disable-next-line no-console
4846
console.log(`Default Settings: ${JSON.stringify(DEFAULT_SETTINGS)}`);
4947

50-
function getDefaultSettings(): firestore.Settings {
48+
function parseTargetBackend(targetBackend: string): TargetBackend {
49+
switch (targetBackend) {
50+
case 'emulator':
51+
return TargetBackend.EMULATOR;
52+
case 'qa':
53+
return TargetBackend.QA;
54+
case 'nightly':
55+
return TargetBackend.NIGHTLY;
56+
case 'prod':
57+
return TargetBackend.PROD;
58+
default:
59+
throw Error('Unknown backend configuration used for integration tests.');
60+
}
61+
}
62+
63+
function getTargetBackend(): TargetBackend {
5164
const karma = typeof __karma__ !== 'undefined' ? __karma__ : undefined;
52-
if (karma && karma.config.firestoreSettings) {
53-
return karma.config.firestoreSettings;
54-
} else {
55-
return USE_EMULATOR ? EMULATOR_FIRESTORE_SETTING : PROD_FIRESTORE_SETTING;
65+
if (karma && karma.config.targetBackend) {
66+
return parseTargetBackend(karma.config.targetBackend);
5667
}
68+
if (process.env.FIRESTORE_TARGET_BACKEND) {
69+
return parseTargetBackend(process.env.FIRESTORE_TARGET_BACKEND);
70+
}
71+
if (process.env.FIRESTORE_EMULATOR_PORT) {
72+
return TargetBackend.EMULATOR;
73+
}
74+
return TargetBackend.PROD;
75+
}
76+
77+
function getFirestoreHost(targetBackend: TargetBackend): string {
78+
switch (targetBackend) {
79+
case TargetBackend.EMULATOR: {
80+
const emulatorPort: string =
81+
process.env.FIRESTORE_EMULATOR_PORT || '8080';
82+
return `localhost:${emulatorPort}`;
83+
}
84+
case TargetBackend.QA:
85+
return 'staging-firestore.sandbox.googleapis.com';
86+
case TargetBackend.NIGHTLY:
87+
return 'test-firestore.sandbox.googleapis.com';
88+
case TargetBackend.PROD:
89+
default:
90+
return 'firestore.googleapis.com';
91+
}
92+
}
93+
94+
function getSslEnabled(targetBackend: TargetBackend): boolean {
95+
return targetBackend !== TargetBackend.EMULATOR;
5796
}
5897

5998
export const DEFAULT_PROJECT_ID = USE_EMULATOR
60-
? EMULATOR_PROJECT_ID
99+
? process.env.FIRESTORE_EMULATOR_PROJECT_ID || 'test-emulator'
61100
: PROJECT_CONFIG.projectId;
62101
export const ALT_PROJECT_ID = 'test-db2';

‎packages/firestore/CONTRIBUTING.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ yarn test:browser --integration
6161

6262
# Run browser integration tests against a Firestore server running on
6363
# localhost:8080.
64-
yarn test:browser --integration --local
64+
yarn test:browser:emulator --integration
6565

6666
# Run all node tests once (unit and integration) against the emulator.
6767
yarn test:node

‎packages/firestore/karma.conf.js

+15-24
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ const karmaBase = require('../../config/karma.base');
1919
const { argv } = require('yargs');
2020

2121
module.exports = function (config) {
22-
const karmaConfig = Object.assign({}, karmaBase, {
22+
const karmaConfig = {
23+
...karmaBase,
2324
browsers: getTestBrowsers(argv),
2425
// files to load into karma
2526
files: getTestFiles(argv),
@@ -30,12 +31,20 @@ module.exports = function (config) {
3031

3132
// frameworks to use
3233
// available frameworks: https://npmjs.org/browse/keyword/karma-adapter
33-
frameworks: ['mocha'],
34+
frameworks: ['mocha']
35+
};
3436

35-
client: Object.assign({}, karmaBase.client, {
36-
firestoreSettings: getFirestoreSettings(argv)
37-
})
38-
});
37+
if (argv.targetBackend) {
38+
karmaConfig.client = {
39+
...karmaConfig.client,
40+
targetBackend: argv.targetBackend
41+
};
42+
} else if (argv.local) {
43+
karmaConfig.client = {
44+
...karmaConfig.client,
45+
targetBackend: 'emulator'
46+
};
47+
}
3948

4049
config.set(karmaConfig);
4150
};
@@ -68,22 +77,4 @@ function getTestBrowsers(argv) {
6877
return browsers;
6978
}
7079

71-
/**
72-
* If the --local argument is passed, returns a {host, ssl} FirestoreSettings
73-
* object that point to localhost instead of production.
74-
*/
75-
function getFirestoreSettings(argv) {
76-
if (argv.local) {
77-
return {
78-
host: 'localhost:8080',
79-
ssl: false
80-
};
81-
} else {
82-
return {
83-
host: 'firestore.googleapis.com',
84-
ssl: true
85-
};
86-
}
87-
}
88-
8980
module.exports.files = getTestFiles(argv);

‎packages/firestore/package.json

+8-8
Original file line numberDiff line numberDiff line change
@@ -21,23 +21,23 @@
2121
"lint": "eslint -c .eslintrc.js '**/*.ts' --ignore-path '../../.gitignore'",
2222
"lint:fix": "eslint --fix -c .eslintrc.js '**/*.ts' --ignore-path '../../.gitignore'",
2323
"prettier": "prettier --write '*.js' '@(lite|src|test)/**/*.ts'",
24-
"test:lite": "node ./scripts/run-tests.js --emulator --platform node_lite --main=lite/index.ts 'test/lite/**/*.test.ts'",
25-
"test:lite:prod": "node ./scripts/run-tests.js --platform node_lite --main=lite/index.ts 'test/lite/**/*.test.ts'",
24+
"test:lite": "ts-node ./scripts/run-tests.ts --emulator --platform node_lite --main=lite/index.ts 'test/lite/**/*.test.ts'",
25+
"test:lite:prod": "ts-node ./scripts/run-tests.ts --platform node_lite --main=lite/index.ts 'test/lite/**/*.test.ts'",
2626
"test:lite:browser": "karma start --single-run --lite",
2727
"test:lite:browser:debug": "karma start --browsers=Chrome --lite --auto-watch",
2828
"test": "run-s lint test:all",
2929
"test:ci": "node ../../scripts/run_tests_in_ci.js -s test:all:ci",
3030
"test:all:ci": "run-p test:browser test:lite:browser test:travis",
3131
"test:all": "run-p test:browser test:lite:browser test:travis test:minified",
3232
"test:browser": "karma start --single-run",
33-
"test:browser:emulator:debug": "karma start --browsers=Chrome --local",
34-
"test:browser:emulator": "karma start --single-run --local",
33+
"test:browser:emulator:debug": "karma start --browsers=Chrome --targetBackend=emulator",
34+
"test:browser:emulator": "karma start --single-run --targetBackend=emulator",
3535
"test:browser:unit": "karma start --single-run --unit",
3636
"test:browser:debug": "karma start --browsers=Chrome --auto-watch",
37-
"test:node": "node ./scripts/run-tests.js --main=test/register.ts --emulator 'test/{,!(browser|lite)/**/}*.test.ts'",
38-
"test:node:prod": "node ./scripts/run-tests.js --main=test/register.ts 'test/{,!(browser|lite)/**/}*.test.ts'",
39-
"test:node:persistence": "node ./scripts/run-tests.js --main=test/register.ts --persistence --emulator 'test/{,!(browser|lite)/**/}*.test.ts'",
40-
"test:node:persistence:prod": "node ./scripts/run-tests.js --main=test/register.ts --persistence 'test/{,!(browser|lite)/**/}*.test.ts'",
37+
"test:node": "ts-node ./scripts/run-tests.ts --main=test/register.ts --emulator 'test/{,!(browser|lite)/**/}*.test.ts'",
38+
"test:node:prod": "ts-node ./scripts/run-tests.ts --main=test/register.ts 'test/{,!(browser|lite)/**/}*.test.ts'",
39+
"test:node:persistence": "ts-node ./scripts/run-tests.ts --main=test/register.ts --persistence --emulator 'test/{,!(browser|lite)/**/}*.test.ts'",
40+
"test:node:persistence:prod": "ts-node ./scripts/run-tests.ts --main=test/register.ts --persistence 'test/{,!(browser|lite)/**/}*.test.ts'",
4141
"test:travis": "ts-node --compiler-options='{\"module\":\"commonjs\"}' ../../scripts/emulator-testing/firestore-test-runner.ts",
4242
"test:minified": "(cd ../../integration/firestore ; yarn test)",
4343
"api-report:main": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' ts-node ../../repo-scripts/prune-dts/extract-public-api.ts --package firestore --packageRoot . --typescriptDts ./dist/firestore/src/index.d.ts --rollupDts ./dist/private.d.ts --untrimmedRollupDts ./dist/internal.d.ts --publicDts ./dist/index.d.ts",

‎packages/firestore/scripts/run-tests.js

-17
This file was deleted.

‎packages/firestore/scripts/run-tests.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ let args = [
5858
];
5959

6060
if (argv.emulator) {
61-
process.env.FIRESTORE_EMULATOR_PORT = '8080';
62-
process.env.FIRESTORE_EMULATOR_PROJECT_ID = 'test-emulator';
61+
process.env.FIRESTORE_TARGET_BACKEND = 'emulator';
6362
}
6463

6564
if (argv.persistence) {

‎packages/firestore/test/integration/browser/webchannel.test.ts

+16-3
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { expect } from 'chai';
2020
import { WebChannelConnection } from '../../../src/platform/browser/webchannel_connection';
2121
import * as api from '../../../src/protos/firestore_proto_api';
2222
import { getDefaultDatabaseInfo } from '../util/internal_helpers';
23-
import { DEFAULT_PROJECT_ID } from '../util/settings';
23+
import { DEFAULT_PROJECT_ID, USE_EMULATOR } from '../util/settings';
2424

2525
/* eslint-disable no-restricted-globals */
2626

@@ -33,7 +33,13 @@ const describeFn =
3333
xdescribe;
3434

3535
describeFn('WebChannel', () => {
36-
it('receives error messages', done => {
36+
// Test does not run on Emulator because emulator does not impose restriction
37+
// on project id. This is likely because emulator does not require creation
38+
// of project or database before using it. However, it might wrong for
39+
// emulator to allow sharing the same stream across different projects and
40+
// databases.
41+
// eslint-disable-next-line no-restricted-properties
42+
(USE_EMULATOR ? it.skip : it)('receives error messages', done => {
3743
const projectId = DEFAULT_PROJECT_ID;
3844
const info = getDefaultDatabaseInfo();
3945
const conn = new WebChannelConnection(info);
@@ -64,9 +70,16 @@ describeFn('WebChannel', () => {
6470

6571
// Wait until we receive data, then send a bad "addTarget" request, causing
6672
// the stream to be closed with an error. In this case, bad means having a
67-
// different database ID.
73+
// different project id.
6874
stream.onMessage(msg => {
6975
if (msg.targetChange) {
76+
// Assertion will fail when additional targets are added. This works so
77+
// long as backend target id counts up from 1. We expect the second
78+
// target to be a bad payload. If this does not fail stream, then we
79+
// might receive message that it was successfully added. The following
80+
// assertion will catch failure to validate data on backend.
81+
expect(msg.targetChange?.targetIds).to.not.include(2);
82+
7083
payload.database = 'projects/some-other-project-id/databases/(default)';
7184
didSendBadPayload = true;
7285
stream.send(payload);

0 commit comments

Comments
 (0)