Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update spectron app handler to pick VSCode window #744

Merged
merged 4 commits into from
Nov 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ test_script:
"Running npm run test:without-system-tests..."
"--------------------------------------------"
npm run test:without-system-tests
#npm run coverage:system-tests
npm run coverage:system-tests
npm run aggregateJUnit
junit-merge -d junit-aggregate -o junit-aggregate.xml
$NpmTestsExitCode = $LastExitCode
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (c) 2018, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { Extension } from 'vscode';
export class MockApexExtension implements Extension<any> {
constructor() {
this.id = 'salesforce.salesforcedx-vscode-apex';
this.extensionPath = 'extension/local/path';
this.isActive = true;
this.exports = new MockJorje();
}
public id: string;
public extensionPath: string;
public isActive: boolean;
public packageJSON: any;
public activate(): Thenable<any> {
return Promise.resolve('activated');
}
public exports: any;
}

class MockJorje {
constructor() {}
public getLineBreakpointInfo(): Promise<{}> {
const response = [
{
uri: '/force-app/main/default/classes/A.cls',
typeref: 'A',
lines: [2, 5, 6, 7]
}
];
return Promise.resolve(response);
}

public isLanguageClientReady() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,35 @@ import {
} from '@salesforce/salesforcedx-apex-debugger/out/src';
import { expect } from 'chai';
import * as sinon from 'sinon';
import * as vscode from 'vscode';
import { DebugConfiguration, extensions, Uri, WorkspaceFolder } from 'vscode';
import { DebugConfigurationProvider } from '../../../src/adapter/debugConfigurationProvider';
import { nls } from '../../../src/messages';
import { MockApexExtension } from './MockApexExtension';

// tslint:disable:no-unused-expression
describe('Configuration provider', () => {
let provider: DebugConfigurationProvider;
let getConfigSpy: sinon.SinonSpy;
const folder: vscode.WorkspaceFolder = {
const folder: WorkspaceFolder = {
name: 'mySfdxProject',
index: 0,
uri: {
fsPath: '/foo'
} as vscode.Uri
} as Uri
};
let mockApexExtension: sinon.SinonStub;

beforeEach(() => {
mockApexExtension = sinon
.stub(extensions, 'getExtension')
.returns(new MockApexExtension());
provider = new DebugConfigurationProvider();
getConfigSpy = sinon.spy(DebugConfigurationProvider, 'getConfig');
});

afterEach(() => {
getConfigSpy.restore();
mockApexExtension.restore();
});

it('Should provide default config', () => {
Expand All @@ -45,7 +51,7 @@ describe('Configuration provider', () => {
requestTypeFilter: [],
entryPointFilter: '',
sfdxProject: '/foo'
} as vscode.DebugConfiguration;
} as DebugConfiguration;

const configs = provider.provideDebugConfigurations(folder);

Expand All @@ -70,6 +76,7 @@ describe('Configuration provider', () => {
expect(config.sfdxProject).to.equals('/foo');
expect(config.workspaceSettings).to.not.equals(undefined);
expect(config.lineBreakpointInfo).to.not.equals(undefined);
expect(mockApexExtension.calledOnce).to.be.true;
} else {
expect.fail(
'Did not get configuration information from resolveDebugConfiguration'
Expand Down Expand Up @@ -106,6 +113,7 @@ describe('Configuration provider', () => {
expect(config.trace).to.equals(true);
expect(config.workspaceSettings).to.not.equals(undefined);
expect(config.lineBreakpointInfo).to.not.equals(undefined);
expect(mockApexExtension.calledOnce).to.be.true;
} else {
expect.fail(
'Did not get configuration information from resolveDebugConfiguration'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (c) 2018, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { Extension } from 'vscode';
export class MockApexExtension implements Extension<any> {
constructor() {
this.id = 'salesforce.salesforcedx-vscode-apex';
this.extensionPath = 'extension/local/path';
this.isActive = true;
this.exports = new MockJorje();
}
public id: string;
public extensionPath: string;
public isActive: boolean;
public packageJSON: any;
public activate(): Thenable<any> {
return Promise.resolve('activated');
}
public exports: any;
}

class MockJorje {
constructor() {}
public getLineBreakpointInfo(): Promise<{}> {
const response = [
{
uri: '/force-app/main/default/classes/A.cls',
typeref: 'A',
lines: [2, 5, 6, 7]
}
];
return Promise.resolve(response);
}

public isLanguageClientReady() {
return true;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
import { DebugConfigurationProvider } from '../../../src/adapter/debugConfigurationProvider';
import { updateLastOpened } from '../../../src/index';
import { nls } from '../../../src/messages';
import { MockJorje } from './MockJorje';
import { MockApexExtension } from './MockApexExtension';

// tslint:disable:no-unused-expression
describe('Configuration provider', () => {
Expand All @@ -36,19 +36,19 @@ describe('Configuration provider', () => {
fsPath: '/foo'
} as Uri
};
let mockApexJorje: sinon.SinonStub;
let mockApexExtension: sinon.SinonStub;

beforeEach(() => {
provider = new DebugConfigurationProvider();
getConfigSpy = sinon.spy(DebugConfigurationProvider, 'getConfig');
mockApexJorje = sinon
mockApexExtension = sinon
.stub(extensions, 'getExtension')
.returns({ exports: new MockJorje() });
.returns(new MockApexExtension());
provider = new DebugConfigurationProvider();
});

afterEach(() => {
getConfigSpy.restore();
mockApexJorje.restore();
mockApexExtension.restore();
});

it('Should provide default config', () => {
Expand Down Expand Up @@ -97,6 +97,7 @@ describe('Configuration provider', () => {
expect(config.trace).to.equals(true);
expect(config.projectPath).to.not.equals(undefined);
expect(config.lineBreakpointInfo).to.not.equals(undefined);
expect(mockApexExtension.calledOnce).to.be.true;
} else {
expect.fail(
'Did not get configuration information from resolveDebugConfiguration'
Expand All @@ -123,6 +124,7 @@ describe('Configuration provider', () => {
expect(config.trace).to.equals(false);
expect(config.projectPath).to.not.equals(undefined);
expect(config.lineBreakpointInfo).to.not.equals(undefined);
expect(mockApexExtension.calledOnce).to.be.true;
} else {
expect.fail(
'Did not get configuration information from resolveDebugConfiguration'
Expand Down
31 changes: 22 additions & 9 deletions packages/system-tests/src/spectron/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,28 @@ export class SpectronApplication {
private async checkWindowReady(): Promise<any> {
await this.webclient.waitUntilWindowLoaded(60000);

// Pick the first workbench window here
// Pick the window that loads VSCode,
// added code to address Spectron limitation in Windows OS (https://github.com/electron/spectron/issues/60)
const count = await this.webclient.getWindowCount();

for (let i = 0; i < count; i++) {
await this.webclient.windowByIndex(i);

if (/bootstrap\/index\.html/.test(await this.webclient.getUrl())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a basic question, but why is the window that we want in focus during the Spectron test going to be named "...bootstrap/index.html"? Is that a Spectron convention?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That has to do with how spectron loads the vscode window, it's normally a file path that represents the local vscode location being used.

I think the path might've changed with vscode moving to a newer electron version (now we get something like electron-browser/workbench/workbench.html) but I was only able to test in Windows and Mac OS so I kept this for now.

break;
if (count > 1) {
for (let i = 0; i < count; i++) {
await this.webclient.windowByIndex(i);
const title = await this.webclient.getTitle();

if (
process.platform === 'win32' &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NodeJs always returns win32 regardless of which windows version is being used (https://nodejs.org/docs/latest-v8.x/api/process.html#process_process_platform).

title !== '' &&
/Visual Studio Code/.test(title)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do without this check but wanted to make sure we look for the vscode window in case the empty terminals start returning a title (currently they return an empty title).

) {
break;
} else if (
/bootstrap\/index\.html/.test(await this.webclient.getUrl())
) {
break;
}
}
} else {
await this.webclient.windowByIndex(0);
}

await this.waitFor(this.spectron.client.getHTML, '.monaco-workbench');
Expand Down Expand Up @@ -177,8 +190,8 @@ export class SpectronApplication {
await this.screenshot.capture();
rej(
`Could not retrieve the element in ${this.testRetry *
this.pollTrials *
this.pollTimeout} seconds. (${JSON.stringify(args)})`
this.pollTrials *
this.pollTimeout} seconds. (${JSON.stringify(args)})`
);
break;
}
Expand Down