Skip to content
This repository has been archived by the owner on Dec 6, 2022. It is now read-only.

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrcrane
mrcrane committed May 21, 2019
1 parent 661a05e commit 0e3916e
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 39 deletions.
31 changes: 14 additions & 17 deletions test/int/stackTrace.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { URL } from 'url';
import { DebugProtocol } from 'vscode-debugprotocol';
import { FrameworkTestContext, TestProjectSpec } from './framework/frameworkTestSupport';
import { puppeteerSuite, puppeteerTest } from './puppeteer/puppeteerSuite';
import { BreakpointsWizard as BreakpointsWizard } from './wizards/breakpoints/breakpointsWizard';
import { BreakpointsWizard } from './wizards/breakpoints/breakpointsWizard';
import { IStackTraceVerifier } from './wizards/breakpoints/implementation/breakpointsAssertions';

const DATA_ROOT = testSetup.DATA_ROOT;
Expand All @@ -22,8 +22,7 @@ interface ExpectedSource {
}

interface ExpectedFrame {
name?: string;
nameRegExp?: RegExp;
name: string | RegExp;
line?: number;
column?: number;
source?: ExpectedSource;
Expand Down Expand Up @@ -73,12 +72,10 @@ function assertSourceMatches(actual: DebugProtocol.Source | undefined, expected:
}

function assertFrameMatches(actual: DebugProtocol.StackFrame, expected: ExpectedFrame, index: number) {
if (expected.name) {
if (typeof expected.name === 'string') {
expect(actual.name).to.equal(expected.name, `Frame ${index} name`);
} else if (expected.nameRegExp) {
expect(actual.name).to.match(expected.nameRegExp, `Frame ${index} name`);
} else {
assert.fail('Not enough information for frame name: set either "name" or "nameRegExp"');
} else if (expected.name instanceof RegExp) {
expect(actual.name).to.match(expected.name, `Frame ${index} name`);
}

expect(actual.line).to.equal(expected.line, `Frame ${index} line`);
Expand Down Expand Up @@ -115,7 +112,7 @@ interface StackTraceValidationConfig {
breakPointLabel: string;
buttonIdToClick: string;
format?: DebugProtocol.StackFrameFormat;
expectedFranes: ExpectedFrame[];
expectedFrames: ExpectedFrame[];
}

async function validateStackTrace(config: StackTraceValidationConfig): Promise<void> {
Expand All @@ -132,7 +129,7 @@ async function validateStackTrace(config: StackTraceValidationConfig): Promise<v
format: config.format,
verify: (stackTraceResponse: DebugProtocol.StackTraceResponse) => {
try {
assertResponseMatches(stackTraceResponse, config.expectedFranes);
assertResponseMatches(stackTraceResponse, config.expectedFrames);
} catch (e) {
const error: assert.AssertionError = e;
error.message += '\nActual stack trace response: \n' + JSON.stringify(stackTraceResponse, null, 2);
Expand All @@ -142,7 +139,7 @@ async function validateStackTrace(config: StackTraceValidationConfig): Promise<v
}
};

await setStateBreakpoint.assertIsHitThenResumeWhen(() => incBtn.click(), {stackTraceVerifier: stackTraceVerifier});
await setStateBreakpoint.assertIsHitThenResumeWhen(() => incBtn.click(), {stackTrace: stackTraceVerifier});
}

puppeteerSuite('Stack Traces', TEST_SPEC, (suiteContext) => {
Expand All @@ -153,7 +150,7 @@ puppeteerSuite('Stack Traces', TEST_SPEC, (suiteContext) => {
breakPointLabel: 'stackTraceBreakpoint',
buttonIdToClick: '#button',
format: undefined,
expectedFranes: [
expectedFrames: [
{ name: '(anonymous function)', line: 11, column: 9, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
{ name: 'evalCallback', line: 12, column: 7, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
{ name: '(eval code)', line: 1, column: 1, source: { evalCode: true }, presentationHint: 'normal'},
Expand All @@ -174,10 +171,10 @@ puppeteerSuite('Stack Traces', TEST_SPEC, (suiteContext) => {
format: {
module: true
},
expectedFranes: [
expectedFrames: [
{ name: '(anonymous function) [app.js]', line: 11, column: 9, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
{ name: 'evalCallback [app.js]', line: 12, column: 7, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
{ nameRegExp: /\(eval code\) \[.*VM.*]/, line: 1, column: 1, source: { evalCode: true }, presentationHint: 'normal'},
{ name: /\(eval code\) \[.*VM.*]/, line: 1, column: 1, source: { evalCode: true }, presentationHint: 'normal'},
{ name: 'timeoutCallback [app.js]', line: 6, column: 5, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
{ name: '[ setTimeout ]', presentationHint: 'label'},
{ name: 'buttonClick [app.js]', line: 2, column: 5, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
Expand All @@ -195,7 +192,7 @@ puppeteerSuite('Stack Traces', TEST_SPEC, (suiteContext) => {
format: {
line: true,
},
expectedFranes: [
expectedFrames: [
{ name: '(anonymous function) Line 11', line: 11, column: 9, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
{ name: 'evalCallback Line 12', line: 12, column: 7, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
{ name: '(eval code) Line 1', line: 1, column: 1, source: { evalCode: true }, presentationHint: 'normal'},
Expand All @@ -220,10 +217,10 @@ puppeteerSuite('Stack Traces', TEST_SPEC, (suiteContext) => {
line: true,
module: true
},
expectedFranes: [
expectedFrames: [
{ name: '(anonymous function) [app.js] Line 11', line: 11, column: 9, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
{ name: 'evalCallback [app.js] Line 12', line: 12, column: 7, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
{ nameRegExp: /\(eval code\) \[.*VM.*] Line 1/, line: 1, column: 1, source: { evalCode: true }, presentationHint: 'normal'},
{ name: /\(eval code\) \[.*VM.*] Line 1/, line: 1, column: 1, source: { evalCode: true }, presentationHint: 'normal'},
{ name: 'timeoutCallback [app.js] Line 6', line: 6, column: 5, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
{ name: '[ setTimeout ]', presentationHint: 'label'},
{ name: 'buttonClick [app.js] Line 2', line: 2, column: 5, source: { fileRelativePath: 'app.js' }, presentationHint: 'normal'},
Expand Down
5 changes: 3 additions & 2 deletions test/int/wizards/breakpoints/fileBreakpointsWizard.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { BreakpointWizard } from './breakpointWizard';
import { InternalFileBreakpointsWizard } from './implementation/internalFileBreakpointsWizard';
import { PromiseOrNot } from 'vscode-chrome-debug-core';
import { wrapWithMethodLogger } from '../../core-v2/chrome/logging/methodsCalledLogger';
import { PauseOnHitCount } from '../../core-v2/chrome/internal/breakpoints/bpActionWhenHit';

export interface IBreakpointOptions {
text: string;
Expand Down Expand Up @@ -30,10 +31,10 @@ export class FileBreakpointsWizard {
}

public async unsetHitCountBreakpoint(options: IHitCountBreakpointOptions): Promise<BreakpointWizard> {
return wrapWithMethodLogger(await this._internal.hitCountBreakpoint({
return wrapWithMethodLogger(await this._internal.breakpoint({
text: options.text,
boundText: options.boundText,
hitCountCondition: options.hitCountCondition,
actionWhenHit: new PauseOnHitCount(options.hitCountCondition),
name: `BP @ ${options.text}`
}));
}
Expand Down
22 changes: 10 additions & 12 deletions test/int/wizards/breakpoints/implementation/breakpointsAssertions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ export interface IStackTraceVerifier {

export interface IVerifications {
variables?: IExpectedVariables;
stackTrace?: IExpectedStackTrace;
stackTraceVerifier?: IStackTraceVerifier;
stackTrace?: IExpectedStackTrace | IStackTraceVerifier;
}

interface IObjectWithLocation {
Expand Down Expand Up @@ -52,20 +51,21 @@ export class BreakpointsAssertions {
format: this._defaultStackFrameFormat,
verify: (stackTraceResponse) => {
expect(stackTraceResponse.success, `Expected the response to the stack trace request to be succesful yet it failed: ${JSON.stringify(stackTraceResponse)}`).to.equal(true);

const stackTraceFrames = stackTraceResponse.body.stackFrames;
expect(stackTraceResponse.body.totalFrames, `The number of stackFrames was different than the value supplied on the totalFrames field`)
.to.equal(stackTraceResponse.body.stackFrames.length);
stackTraceResponse.body.stackFrames.forEach(frame => {
.to.equal(stackTraceFrames.length);
stackTraceFrames.forEach(frame => {
// Warning: We don't currently validate frame.source.path
expect(frame.source).not.to.equal(undefined);
const expectedSourceNameAndLine = ` [${frame.source!.name}] Line ${frame.line}`;
expect(frame.name, 'Expected the formatted name to match the source name and line supplied as individual attributes').to.endsWith(expectedSourceNameAndLine);
});

const stackTrace = stackTraceResponse.body.stackFrames;

const formattedExpectedStackTrace = expectedStackTrace.replace(/^\s+/gm, ''); // Remove the white space we put at the start of the lines to make the stack trace align with the code
this.applyIgnores(formattedExpectedStackTrace, stackTrace);
const actualStackTrace = this.extractStackTrace(stackTrace);
this.applyIgnores(formattedExpectedStackTrace, stackTraceFrames);
const actualStackTrace = this.extractStackTrace(stackTraceFrames);
assert.equal(actualStackTrace, formattedExpectedStackTrace, `Expected the stack trace when hitting ${breakpoint} to be:\n${formattedExpectedStackTrace}\nyet it is:\n${actualStackTrace}`);
}
};
Expand Down Expand Up @@ -95,12 +95,10 @@ export class BreakpointsAssertions {
await this._breakpointsWizard.waitUntilPaused(breakpoint);

let stackTraceVerifier: IStackTraceVerifier | undefined = undefined;
if (verifications.stackTrace && verifications.stackTraceVerifier) {
assert.fail('Cannot set both verifications.stackTrace and verifications.stackTraceVerifier. Choose at most one.');
} else if (verifications.stackTrace) {
if (typeof verifications.stackTrace === 'string') {
stackTraceVerifier = this.getDefaultStackTraceVerifier(breakpoint, verifications.stackTrace);
} else if (verifications.stackTraceVerifier) {
stackTraceVerifier = verifications.stackTraceVerifier;
} else if (typeof verifications.stackTrace === 'object') {
stackTraceVerifier = verifications.stackTrace;
}

let stackFrameFormat: DebugProtocol.StackFrameFormat | undefined = undefined;
Expand Down
12 changes: 4 additions & 8 deletions test/int/wizards/breakpoints/implementation/internalFileBreakpointsWizard.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ExtendedDebugClient } from 'vscode-chrome-debug-core-testsupport';
import { findPositionOfTextInFile } from '../../../utils/findPositionOfTextInFile';
import { DebugProtocol } from 'vscode-debugprotocol';
import { PauseOnHitCount, AlwaysPause } from '../../../core-v2/chrome/internal/breakpoints/bpActionWhenHit';
import { AlwaysPause, IBPActionWhenHit } from '../../../core-v2/chrome/internal/breakpoints/bpActionWhenHit';
import { BreakpointWizard } from '../breakpointWizard';
import { ValidatedMap } from '../../../core-v2/chrome/collections/validatedMap';
import { FileBreakpointsWizard } from '../fileBreakpointsWizard';
Expand Down Expand Up @@ -48,16 +48,12 @@ export class InternalFileBreakpointsWizard {

public constructor(private readonly _breakpointsWizard: BreakpointsWizard, public readonly client: ExtendedDebugClient, public readonly filePath: string) { }

public async breakpoint(options: { text: string, name: string, boundText?: string }): Promise<BreakpointWizard> {
public async breakpoint(options: { name: string, text: string, boundText?: string, actionWhenHit?: IBPActionWhenHit}) {
const position = await findPositionOfTextInFile(this.filePath, options.text);
const boundPosition = options.boundText ? await findPositionOfTextInFile(this.filePath, options.boundText) : position;
return new BreakpointWizard(this, position, new AlwaysPause(), options.name, boundPosition);
}
const actionWhenHit = options.actionWhenHit || new AlwaysPause();

public async hitCountBreakpoint(options: { text: string; hitCountCondition: string; name: string, boundText?: string }): Promise<BreakpointWizard> {
const position = await findPositionOfTextInFile(this.filePath, options.text);
const boundPosition = options.boundText ? await findPositionOfTextInFile(this.filePath, options.boundText) : position;
return new BreakpointWizard(this, position, new PauseOnHitCount(options.hitCountCondition), options.name, boundPosition);
return new BreakpointWizard(this, position, actionWhenHit, options.name, boundPosition);
}

public async set(breakpointWizard: BreakpointWizard): Promise<void> {
Expand Down

0 comments on commit 0e3916e

Please sign in to comment.