Skip to content

Commit f2dd5c0

Browse files
authored
Fix Saucelabs tests for Firestore unit tests except IE (#3095)
1 parent b57d68a commit f2dd5c0

File tree

9 files changed

+59
-37
lines changed

9 files changed

+59
-37
lines changed

config/karma.saucelabs.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ const packageConfigs = {
4949
messaging: {
5050
// Messaging currently only supports these browsers.
5151
browsers: ['Chrome_Windows', 'Firefox_Windows', 'Edge_Windows']
52+
},
53+
firestore: {
54+
browsers: [
55+
'Chrome_Windows',
56+
'Firefox_Windows',
57+
'Edge_Windows',
58+
'Safari_macOS'
59+
]
5260
}
5361
};
5462

@@ -170,7 +178,8 @@ module.exports = function(config) {
170178
'packages/polyfill/index.ts': ['webpack', 'sourcemap'],
171179
'**/test/**/*.ts': ['webpack', 'sourcemap'],
172180
'**/*.test.ts': ['webpack', 'sourcemap'],
173-
'packages/firestore/test/**/bootstrap.ts': ['webpack', 'babel'],
181+
// Restore when ready to run Firestore unit tests in IE.
182+
// 'packages/firestore/test/**/bootstrap.ts': ['webpack', 'babel'],
174183
'integration/**/namespace.*': ['webpack', 'babel', 'sourcemap']
175184
},
176185

package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
"test:changed": "node scripts/run_changed.js",
4141
"test:setup": "node tools/config.js",
4242
"test:saucelabs": "node scripts/run_saucelabs.js",
43-
"test:saucelabs:single": "karma start config/karma.saucelabs.js --single-run",
4443
"docgen:js": "node scripts/docgen/generate-docs.js --api js",
4544
"docgen:node": "node scripts/docgen/generate-docs.js --api node",
4645
"docgen": "yarn docgen:js; yarn docgen:node",

packages/firestore/karma.conf.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @license
3-
* Copyright 2017 Google Inc.
3+
* Copyright 2017 Google LLC
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -15,8 +15,6 @@
1515
* limitations under the License.
1616
*/
1717

18-
const karma = require('karma');
19-
const path = require('path');
2018
const karmaBase = require('../../config/karma.base');
2119
const { argv } = require('yargs');
2220

packages/firestore/src/util/async_queue.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ export class AsyncQueue {
358358
.catch((error: FirestoreError) => {
359359
this.failure = error;
360360
this.operationInProgress = false;
361-
const message = error.stack || error.message || '';
361+
const message = getMessageOrStack(error);
362362
logError('INTERNAL UNHANDLED ERROR: ', message);
363363

364364
// Re-throw the error so that this.tail becomes a rejected Promise and
@@ -411,10 +411,7 @@ export class AsyncQueue {
411411

412412
private verifyNotFailed(): void {
413413
if (this.failure) {
414-
fail(
415-
'AsyncQueue is already failed: ' +
416-
(this.failure.stack || this.failure.message)
417-
);
414+
fail('AsyncQueue is already failed: ' + getMessageOrStack(this.failure));
418415
}
419416
}
420417

@@ -515,3 +512,20 @@ export function wrapInUserErrorIfRecoverable(
515512
throw e;
516513
}
517514
}
515+
516+
/**
517+
* Chrome includes Error.message in Error.stack. Other browsers do not.
518+
* This returns expected output of message + stack when available.
519+
* @param error Error or FirestoreError
520+
*/
521+
function getMessageOrStack(error: Error): string {
522+
let message = error.message || '';
523+
if (error.stack) {
524+
if (error.stack.includes(error.message)) {
525+
message = error.stack;
526+
} else {
527+
message = error.message + '\n' + error.stack;
528+
}
529+
}
530+
return message;
531+
}

packages/firestore/test/unit/core/event_manager.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,10 @@ describe('QueryListener', () => {
224224
const query = Query.atPath(path('rooms/Eros'));
225225

226226
const listener = queryListener(query, [], events);
227+
const error = new Error('bad');
227228

228-
listener.onError(Error('bad'));
229-
expect(events[0]).to.deep.equal(new Error('bad'));
229+
listener.onError(error);
230+
expect(events[0]).to.deep.equal(error);
230231
});
231232

232233
it('raises event for empty collection after sync', () => {

packages/firestore/test/unit/util/async_queue.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { getLogLevel, LogLevel, setLogLevel } from '../../../src/util/log';
2424
import { Deferred, Rejecter, Resolver } from '../../../src/util/promise';
2525
import { fail } from '../../../src/util/assert';
2626
import { IndexedDbTransactionError } from '../../../src/local/simple_db';
27+
import { isSafari } from '@firebase/util';
2728

2829
use(chaiAsPromised);
2930

@@ -137,7 +138,9 @@ describe('AsyncQueue', () => {
137138
});
138139
});
139140

140-
it('can schedule ops in the future', async () => {
141+
// Flaky on Safari.
142+
// eslint-disable-next-line no-restricted-properties
143+
(isSafari() ? it.skip : it)('can schedule ops in the future', async () => {
141144
const queue = new AsyncQueue();
142145
const completedSteps: number[] = [];
143146
const doStep = (n: number): Promise<number> =>

packages/firestore/test/util/test_platform.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -271,12 +271,3 @@ export class TestPlatform implements Platform {
271271
return this.basePlatform.randomBytes(nBytes);
272272
}
273273
}
274-
275-
/** Returns true if we are running under Node. */
276-
export function isNode(): boolean {
277-
return (
278-
typeof process !== 'undefined' &&
279-
process.title !== undefined &&
280-
process.title.indexOf('node') !== -1
281-
);
282-
}

packages/util/src/environment.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,3 +125,12 @@ export function isUWP(): boolean {
125125
export function isNodeSdk(): boolean {
126126
return CONSTANTS.NODE_CLIENT === true || CONSTANTS.NODE_ADMIN === true;
127127
}
128+
129+
/** Returns true if we are running in Safari. */
130+
export function isSafari(): boolean {
131+
return (
132+
!isNode() &&
133+
navigator.userAgent.includes('Safari') &&
134+
!navigator.userAgent.includes('Chrome')
135+
);
136+
}

scripts/run_saucelabs.js

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @license
3-
* Copyright 2019 Google Inc.
3+
* Copyright 2019 Google LLC
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -44,8 +44,7 @@ const testFiles = configFiles.length
4444
// Get CI build number or generate one if running locally.
4545
const buildNumber =
4646
process.env.TRAVIS_BUILD_NUMBER ||
47-
// GitHub Actions does not have a build number, but the feature has been requested.
48-
process.env.GITHUB_SHA ||
47+
process.env.GITHUB_RUN_ID ||
4948
`local_${process.env.USER}_${new Date().getTime()}`;
5049

5150
/**
@@ -67,26 +66,25 @@ async function runTest(testFile) {
6766
});
6867
}
6968
}
69+
return runKarma(testFile);
70+
}
7071

71-
const promise = spawn('yarn', [
72-
'test:saucelabs:single',
72+
async function runKarma(testFile) {
73+
const karmaArgs = [
74+
'karma',
75+
'start',
76+
'config/karma.saucelabs.js',
77+
'--single-run',
7378
'--testConfigFile',
7479
testFile,
7580
'--buildNumber',
7681
buildNumber
77-
]);
82+
];
83+
84+
const promise = spawn('npx', karmaArgs, { stdio: 'inherit' });
7885
const childProcess = promise.childProcess;
7986
let exitCode = 0;
8087

81-
childProcess.stdout.on('data', data => {
82-
console.log(`[${testFile}]:`, data.toString());
83-
});
84-
85-
// Lerna's normal output goes to stderr for some reason.
86-
childProcess.stderr.on('data', data => {
87-
console.log(`[${testFile}]:`, data.toString());
88-
});
89-
9088
// Capture exit code of this single package test run
9189
childProcess.on('exit', code => {
9290
exitCode = code;

0 commit comments

Comments
 (0)