Skip to content

Commit b6166c9

Browse files
dgozmandgozman
authored
chore(testrunner): introduce Location class (#1585)
Drive-by: fix an edge when testing continued after termination.
1 parent c49b856 commit b6166c9

File tree

8 files changed

+120
-66
lines changed

8 files changed

+120
-66
lines changed

‎test/playwright.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ module.exports.describe = ({testRunner, product, playwrightPath}) => {
144144
afterEach(async (state, test) => {
145145
if (state.browser.contexts().length !== 0) {
146146
if (test.result === 'ok')
147-
console.warn(`\nWARNING: test "${test.fullName}" (${test.location.fileName}:${test.location.lineNumber}) did not close all created contexts!\n`);
147+
console.warn(`\nWARNING: test "${test.fullName()}" (${test.location()}) did not close all created contexts!\n`);
148148
await Promise.all(state.browser.contexts().map(context => context.close()));
149149
}
150150
await state.tearDown();

‎test/utils.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,8 @@ const utils = module.exports = {
254254
const url = `https://github.com/Microsoft/playwright/blob/${sha}/${testpath}#L${test.location.lineNumber}`;
255255
dashboard.reportTestResult({
256256
testId: test.testId,
257-
name: test.location.fileName + ':' + test.location.lineNumber,
258-
description: test.fullName,
257+
name: test.location().toString(),
258+
description: test.fullName(),
259259
url,
260260
result: test.result,
261261
});
@@ -275,7 +275,7 @@ const utils = module.exports = {
275275
const testId = testIdComponents.join('>');
276276
const clashingTest = testIds.get(testId);
277277
if (clashingTest)
278-
throw new Error(`Two tests with clashing IDs: ${test.location.fileName}:${test.location.lineNumber} and ${clashingTest.location.fileName}:${clashingTest.location.lineNumber}`);
278+
throw new Error(`Two tests with clashing IDs: ${test.location()} and ${clashingTest.location()}`);
279279
testIds.set(testId, test);
280280
test.testId = testId;
281281
}

‎utils/testrunner/Location.js

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Copyright 2017 Google Inc. All rights reserved.
3+
* Modifications copyright (c) Microsoft Corporation.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
const path = require('path');
19+
20+
class Location {
21+
constructor() {
22+
this._fileName = '';
23+
this._filePath = '';
24+
this._lineNumber = 0;
25+
this._columnNumber = 0;
26+
}
27+
28+
fileName() {
29+
return this._fileName;
30+
}
31+
32+
filePath() {
33+
return this._filePath;
34+
}
35+
36+
lineNumber() {
37+
return this._lineNumber;
38+
}
39+
40+
columnNumber() {
41+
return this._columnNumber;
42+
}
43+
44+
toString() {
45+
return this._fileName + ':' + this._lineNumber;
46+
}
47+
48+
toDetailedString() {
49+
return this._fileName + ':' + this._lineNumber + ':' + this._columnNumber;
50+
}
51+
52+
static getCallerLocation(filename) {
53+
const error = new Error();
54+
const stackFrames = error.stack.split('\n').slice(1);
55+
const location = new Location();
56+
// Find first stackframe that doesn't point to this file.
57+
for (let frame of stackFrames) {
58+
frame = frame.trim();
59+
if (!frame.startsWith('at '))
60+
return null;
61+
if (frame.endsWith(')')) {
62+
const from = frame.indexOf('(');
63+
frame = frame.substring(from + 1, frame.length - 1);
64+
} else {
65+
frame = frame.substring('at '.length);
66+
}
67+
68+
const match = frame.match(/^(.*):(\d+):(\d+)$/);
69+
if (!match)
70+
return null;
71+
const filePath = match[1];
72+
if (filePath === __filename || filePath === filename)
73+
continue;
74+
75+
location._filePath = filePath;
76+
location._fileName = filePath.split(path.sep).pop();
77+
location._lineNumber = parseInt(match[2], 10);
78+
location._columnNumber = parseInt(match[3], 10);
79+
return location;
80+
}
81+
return location;
82+
}
83+
}
84+
85+
module.exports = Location;

‎utils/testrunner/Matchers.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
const {getCallerLocation} = require('./utils.js');
17+
const Location = require('./Location.js');
1818
const colors = require('colors/safe');
1919
const Diff = require('text-diff');
2020

@@ -40,7 +40,7 @@ class MatchError extends Error {
4040
super(message);
4141
this.name = this.constructor.name;
4242
this.formatter = formatter;
43-
this.location = getCallerLocation(__filename);
43+
this.location = Location.getCallerLocation(__filename);
4444
Error.captureStackTrace(this, this.constructor);
4545
}
4646
}

‎utils/testrunner/Reporter.js

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -198,21 +198,22 @@ class Reporter {
198198
} else if (testRun.result() === 'failed') {
199199
console.log(`${prefix} ${colors.red('[FAIL]')} ${test.fullName()} (${formatLocation(test.location())})`);
200200
if (testRun.error() instanceof MatchError) {
201-
let lines = this._filePathToLines.get(testRun.error().location.filePath);
201+
const location = testRun.error().location;
202+
let lines = this._filePathToLines.get(location.filePath());
202203
if (!lines) {
203204
try {
204-
lines = fs.readFileSync(testRun.error().location.filePath, 'utf8').split('\n');
205+
lines = fs.readFileSync(location.filePath(), 'utf8').split('\n');
205206
} catch (e) {
206207
lines = [];
207208
}
208-
this._filePathToLines.set(testRun.error().location.filePath, lines);
209+
this._filePathToLines.set(location.filePath(), lines);
209210
}
210-
const lineNumber = testRun.error().location.lineNumber;
211+
const lineNumber = location.lineNumber();
211212
if (lineNumber < lines.length) {
212213
const lineNumberLength = (lineNumber + 1 + '').length;
213-
const FROM = Math.max(test.location().lineNumber - 1, lineNumber - 5);
214+
const FROM = Math.max(test.location().lineNumber() - 1, lineNumber - 5);
214215
const snippet = lines.slice(FROM, lineNumber).map((line, index) => ` ${(FROM + index + 1 + '').padStart(lineNumberLength, ' ')} | ${line}`).join('\n');
215-
const pointer = ` ` + ' '.repeat(lineNumberLength) + ' ' + '~'.repeat(testRun.error().location.columnNumber - 1) + '^';
216+
const pointer = ` ` + ' '.repeat(lineNumberLength) + ' ' + '~'.repeat(location.columnNumber() - 1) + '^';
216217
console.log('\n' + snippet + '\n' + colors.grey(pointer) + '\n');
217218
}
218219
console.log(padLines(testRun.error().formatter(), 4));
@@ -228,10 +229,10 @@ class Reporter {
228229
console.log(' Stack:');
229230
let stack = testRun.error().stack;
230231
// Highlight first test location, if any.
231-
const match = stack.match(new RegExp(test.location().filePath + ':(\\d+):(\\d+)'));
232+
const match = stack.match(new RegExp(test.location().filePath() + ':(\\d+):(\\d+)'));
232233
if (match) {
233234
const [, line, column] = match;
234-
const fileName = `${test.location().fileName}:${line}:${column}`;
235+
const fileName = `${test.location().fileName()}:${line}:${column}`;
235236
stack = stack.substring(0, match.index) + stack.substring(match.index).replace(fileName, colors.yellow(fileName));
236237
}
237238
console.log(padLines(stack, 4));
@@ -249,7 +250,7 @@ class Reporter {
249250
function formatLocation(location) {
250251
if (!location)
251252
return '';
252-
return colors.yellow(`${location.fileName}:${location.lineNumber}:${location.columnNumber}`);
253+
return colors.yellow(`${location.toDetailedString()}`);
253254
}
254255

255256
function padLines(text, spaces = 0) {

‎utils/testrunner/TestRunner.js

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/**
22
* Copyright 2017 Google Inc. All rights reserved.
3+
* Modifications copyright (c) Microsoft Corporation.
34
*
45
* Licensed under the Apache License, Version 2.0 (the "License");
56
* you may not use this file except in compliance with the License.
@@ -17,7 +18,7 @@
1718
const EventEmitter = require('events');
1819
const {SourceMapSupport} = require('./SourceMapSupport');
1920
const debug = require('debug');
20-
const {getCallerLocation} = require('./utils');
21+
const Location = require('./Location');
2122

2223
const INFINITE_TIMEOUT = 100000000;
2324
const TimeoutError = new Error('Timeout');
@@ -53,7 +54,7 @@ const TestResult = {
5354
};
5455

5556
function createHook(callback, name) {
56-
const location = getCallerLocation(__filename);
57+
const location = Location.getCallerLocation(__filename);
5758
return { name, body: callback, location };
5859
}
5960

@@ -456,14 +457,13 @@ class TestWorker {
456457
this._runningHookTerminate = null;
457458

458459
if (error) {
459-
const locationString = `${hook.location.fileName}:${hook.location.lineNumber}:${hook.location.columnNumber}`;
460460
if (testRun && testRun._result !== TestResult.Terminated) {
461461
// Prefer terminated result over any hook failures.
462462
testRun._result = error === TerminatedError ? TestResult.Terminated : TestResult.Crashed;
463463
}
464464
let message;
465465
if (error === TimeoutError) {
466-
message = `${locationString} - Timeout Exceeded ${timeout}ms while running "${hook.name}" in "${fullName}"`;
466+
message = `${hook.location.toDetailedString()} - Timeout Exceeded ${timeout}ms while running "${hook.name}" in "${fullName}"`;
467467
error = null;
468468
} else if (error === TerminatedError) {
469469
// Do not report termination details - it's just noise.
@@ -472,7 +472,7 @@ class TestWorker {
472472
} else {
473473
if (error.stack)
474474
await this._testPass._runner._sourceMapSupport.rewriteStackTraceWithSourceMaps(error);
475-
message = `${locationString} - FAILED while running "${hook.name}" in suite "${fullName}": `;
475+
message = `${hook.location.toDetailedString()} - FAILED while running "${hook.name}" in suite "${fullName}": `;
476476
}
477477
await this._didFailHook(hook, fullName, message, error);
478478
if (testRun)
@@ -497,26 +497,26 @@ class TestWorker {
497497
}
498498

499499
async _willStartTestBody(testRun) {
500-
debug('testrunner:test')(`[${this._workerId}] starting "${testRun.test().fullName()}" (${testRun.test().location().fileName + ':' + testRun.test().location().lineNumber})`);
500+
debug('testrunner:test')(`[${this._workerId}] starting "${testRun.test().fullName()}" (${testRun.test().location()})`);
501501
}
502502

503503
async _didFinishTestBody(testRun) {
504-
debug('testrunner:test')(`[${this._workerId}] ${testRun._result.toUpperCase()} "${testRun.test().fullName()}" (${testRun.test().location().fileName + ':' + testRun.test().location().lineNumber})`);
504+
debug('testrunner:test')(`[${this._workerId}] ${testRun._result.toUpperCase()} "${testRun.test().fullName()}" (${testRun.test().location()})`);
505505
}
506506

507507
async _willStartHook(hook, fullName) {
508-
debug('testrunner:hook')(`[${this._workerId}] "${hook.name}" started for "${fullName}" (${hook.location.fileName + ':' + hook.location.lineNumber})`);
508+
debug('testrunner:hook')(`[${this._workerId}] "${hook.name}" started for "${fullName}" (${hook.location})`);
509509
}
510510

511511
async _didFailHook(hook, fullName, message, error) {
512-
debug('testrunner:hook')(`[${this._workerId}] "${hook.name}" FAILED for "${fullName}" (${hook.location.fileName + ':' + hook.location.lineNumber})`);
512+
debug('testrunner:hook')(`[${this._workerId}] "${hook.name}" FAILED for "${fullName}" (${hook.location})`);
513513
if (message)
514514
this._testPass._result.addError(message, error, this);
515515
this._testPass._result.setResult(TestResult.Crashed, message);
516516
}
517517

518518
async _didCompleteHook(hook, fullName) {
519-
debug('testrunner:hook')(`[${this._workerId}] "${hook.name}" OK for "${fullName}" (${hook.location.fileName + ':' + hook.location.lineNumber})`);
519+
debug('testrunner:hook')(`[${this._workerId}] "${hook.name}" OK for "${fullName}" (${hook.location})`);
520520
}
521521

522522
async shutdown() {
@@ -581,7 +581,7 @@ class TestPass {
581581
async _runWorker(testRunIndex, testRuns, parallelIndex) {
582582
let worker = new TestWorker(this, this._nextWorkerId++, parallelIndex);
583583
this._workers[parallelIndex] = worker;
584-
while (!worker._terminating) {
584+
while (!this._terminating) {
585585
let skipped = 0;
586586
while (skipped < testRuns.length && testRuns[testRunIndex]._result !== null) {
587587
testRunIndex = (testRunIndex + 1) % testRuns.length;
@@ -613,6 +613,7 @@ class TestPass {
613613

614614
async _terminate(result, message, force, error) {
615615
debug('testrunner')(`TERMINATED result = ${result}, message = ${message}`);
616+
this._terminating = true;
616617
for (const worker of this._workers)
617618
worker.terminate(force /* terminateHooks */);
618619
this._result.setResult(result, message);
@@ -638,8 +639,7 @@ class TestRunner extends EventEmitter {
638639
} = options;
639640
this._crashIfTestsAreFocusedOnCI = crashIfTestsAreFocusedOnCI;
640641
this._sourceMapSupport = new SourceMapSupport();
641-
const dummyLocation = { fileName: '', filePath: '', lineNumber: 0, columnNumber: 0 };
642-
this._rootSuite = new Suite(null, '', dummyLocation);
642+
this._rootSuite = new Suite(null, '', new Location());
643643
this._currentSuite = this._rootSuite;
644644
this._tests = [];
645645
this._suites = [];
@@ -670,7 +670,7 @@ class TestRunner extends EventEmitter {
670670

671671
_suiteBuilder(callbacks) {
672672
return new Proxy((name, callback, ...suiteArgs) => {
673-
const location = getCallerLocation(__filename);
673+
const location = Location.getCallerLocation(__filename);
674674
const suite = new Suite(this._currentSuite, name, location);
675675
for (const { callback, args } of callbacks)
676676
callback(suite, ...args);
@@ -692,7 +692,7 @@ class TestRunner extends EventEmitter {
692692

693693
_testBuilder(callbacks) {
694694
return new Proxy((name, callback) => {
695-
const location = getCallerLocation(__filename);
695+
const location = Location.getCallerLocation(__filename);
696696
const test = new Test(this._currentSuite, name, callback, location);
697697
test.setTimeout(this._timeout);
698698
for (const { callback, args } of callbacks)

‎utils/testrunner/test/testrunner.spec.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ module.exports.addTests = function({testRunner, expect}) {
2121
expect(test.fullName()).toBe('uno');
2222
expect(test.focused()).toBe(false);
2323
expect(test.skipped()).toBe(false);
24-
expect(test.location().filePath).toEqual(__filename);
25-
expect(test.location().fileName).toEqual('testrunner.spec.js');
26-
expect(test.location().lineNumber).toBeTruthy();
27-
expect(test.location().columnNumber).toBeTruthy();
24+
expect(test.location().filePath()).toEqual(__filename);
25+
expect(test.location().fileName()).toEqual('testrunner.spec.js');
26+
expect(test.location().lineNumber()).toBeTruthy();
27+
expect(test.location().columnNumber()).toBeTruthy();
2828
});
2929
it('should run a test', async() => {
3030
const t = newTestRunner();

‎utils/testrunner/utils.js

Lines changed: 0 additions & 32 deletions
This file was deleted.

0 commit comments

Comments
 (0)