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

[TS-SELENIUM] Cover the "Use Java IDE features and the inner loop" step from "Happy path" scenario #13615

Merged
merged 19 commits into from
Jul 5, 2019
Merged
Show file tree
Hide file tree
Changes from 6 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
3 changes: 3 additions & 0 deletions deploy/kubernetes/helm/che/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,6 @@ data:
JAEGER_SAMPLER_PARAM: "1"
JAEGER_REPORTER_MAX_QUEUE_SIZE: "10000"
CHE_METRICS_ENABLED: {{ .Values.global.metricsEnabled | quote }}
CHE_WORKSPACE_JAVA__OPTIONS: "-Xmx2000m"
CHE_WORKSPACE_MAVEN__OPTIONS: "-Xmx20000m"
CHE_INFRA_KUBERNETES_WORKSPACE__START__TIMEOUT__MIN: "15"
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we tried that without this timeout? (Just with default one?) I would prefer having default one and put this extended one just in case the default proves to be not sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this variables each launch fails with "heap space" error

Copy link
Contributor

Choose a reason for hiding this comment

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

I may have not been completely clear here... I was talking just about CHE_INFRA_KUBERNETES_WORKSPACE__START__TIMEOUT__MIN. This one is not affecting command execution (build inside workspace) in any way.

20 changes: 18 additions & 2 deletions e2e/driver/CheReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@
import * as mocha from 'mocha';
import { IDriver } from './IDriver';
import { e2eContainer } from '../inversify.config';
import { TYPES } from '../inversify.types';
import { TYPES, CLASSES } from '../inversify.types';
import * as fs from 'fs';
import { TestConstants } from '../TestConstants';
import { logging } from 'selenium-webdriver';
import { DriverHelper } from '../utils/DriverHelper';

const driver: IDriver = e2eContainer.get(TYPES.Driver);
const driverHelper: DriverHelper = e2eContainer.get(CLASSES.DriverHelper);

class CheReporter extends mocha.reporters.Spec {

Expand Down Expand Up @@ -65,6 +68,8 @@ class CheReporter extends mocha.reporters.Spec {
const testReportDirPath: string = `${reportDirPath}/${testFullTitle}`;
const screenshotFileName: string = `${testReportDirPath}/screenshot-${testTitle}.png`;
const pageSourceFileName: string = `${testReportDirPath}/pagesource-${testTitle}.html`;
const browserLogsFileName: string = `${testReportDirPath}/browserlogs-${testTitle}.txt`;


// create reporter dir if not exist
const reportDirExists: boolean = fs.existsSync(reportDirPath);
Expand All @@ -91,8 +96,19 @@ class CheReporter extends mocha.reporters.Spec {
const pageSourceStream = fs.createWriteStream(pageSourceFileName);
pageSourceStream.write(new Buffer(pageSource));
pageSourceStream.end();
});

// take browser console logs and write to file
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice addition. +1 ;-)

const browserLogsEntries: logging.Entry[] = await driverHelper.getDriver().manage().logs().get('browser');
let browserLogs: string = '';

browserLogsEntries.forEach(log => {
browserLogs += `\"${log.level}\" \"${log.type}\" \"${log.message}\"\n`;
});

const browserLogsStream = fs.createWriteStream(browserLogsFileName);
browserLogsStream.write(new Buffer(browserLogs));
browserLogsStream.end();
});
}
}

Expand Down
6 changes: 6 additions & 0 deletions e2e/inversify.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ import { QuickOpenContainer } from './pageobjects/ide/QuickOpenContainer';
import { PreviewWidget } from './pageobjects/ide/PreviewWidget';
import { GitHubPlugin } from './pageobjects/ide/GitHubPlugin';
import { RightToolbar } from './pageobjects/ide/RightToolbar';
import { Terminal } from './pageobjects/ide/Terminal';
import { DebugView } from './pageobjects/ide/DebugView';
import { WarningDialog } from './pageobjects/ide/WarningDialog';

const e2eContainer = new Container();

Expand All @@ -49,5 +52,8 @@ e2eContainer.bind<QuickOpenContainer>(CLASSES.QuickOpenContainer).to(QuickOpenCo
e2eContainer.bind<PreviewWidget>(CLASSES.PreviewWidget).to(PreviewWidget).inSingletonScope();
e2eContainer.bind<GitHubPlugin>(CLASSES.GitHubPlugin).to(GitHubPlugin).inSingletonScope();
e2eContainer.bind<RightToolbar>(CLASSES.RightToolbar).to(RightToolbar).inSingletonScope();
e2eContainer.bind<Terminal>(CLASSES.Terminal).to(Terminal).inSingletonScope();
e2eContainer.bind<DebugView>(CLASSES.DebugView).to(DebugView).inSingletonScope();
e2eContainer.bind<WarningDialog>(CLASSES.WarningDialog).to(WarningDialog).inSingletonScope();

export { e2eContainer };
5 changes: 4 additions & 1 deletion e2e/inversify.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ const CLASSES = {
QuickOpenContainer: 'QuickOpenContainer',
PreviewWidget: 'PreviewWidget',
GitHubPlugin: 'GitHubPlugin',
RightToolbar: 'RightToolbar'
RightToolbar: 'RightToolbar',
Terminal: 'Terminal',
DebugView: 'DebugView',
WarningDialog: 'WarningDialog'
};

export { TYPES, CLASSES };
4 changes: 2 additions & 2 deletions e2e/pageobjects/dashboard/NewWorkspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ export class NewWorkspace {
}

async waitPageAbsence(timeout: number = TestConstants.TS_SELENIUM_LOAD_PAGE_TIMEOUT) {
await this.driverHelper.waitDisappearanceTestWithTimeout(By.css(NewWorkspace.NAME_FIELD_CSS), timeout);
await this.driverHelper.waitDisappearanceTestWithTimeout(By.css(NewWorkspace.TITLE_CSS), timeout);
await this.driverHelper.waitDisappearanceWithTimeout(By.css(NewWorkspace.NAME_FIELD_CSS), timeout);
await this.driverHelper.waitDisappearanceWithTimeout(By.css(NewWorkspace.TITLE_CSS), timeout);
}

async createWorkspaceAndProceedEditing(workspaceName: string, dataStackId: string, sampleName: string) {
Expand Down
47 changes: 47 additions & 0 deletions e2e/pageobjects/ide/DebugView.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*********************************************************************
* Copyright (c) 2019 Red Hat, Inc.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
**********************************************************************/
import { inject, injectable } from 'inversify';
import { CLASSES } from '../../inversify.types';
import { DriverHelper } from '../../utils/DriverHelper';
import { By, Key } from 'selenium-webdriver';
import { Ide } from './Ide';


@injectable()
export class DebugView {
constructor(@inject(CLASSES.DriverHelper) private readonly driverHelper: DriverHelper,
@inject(CLASSES.Ide) private readonly ide: Ide) { }

async clickOnDebugConfigurationDropDown() {
await this.driverHelper.waitAndClick(By.css('select.debug-configuration'));
}

async clickOnDebugConfigurationItem(itemText: string) {
const configurationItemLocator: By = By.xpath(`//select[@class='debug-configuration']//option[text()=\'${itemText}\']`);

// for ensure that drop-down list has been opened
await this.driverHelper.wait(2000);
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't these "hard" waits be replaced by some conditional waits? I think having "dumb" sleeps in test code is not a good practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, it is not critical because it works properly even without this sleep. This is my overcautiousness for CI launches

Copy link
Contributor

Choose a reason for hiding this comment

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

I strongly believe any sleep like this should be either removed, or replaced by conditional wait.
These three waits in this method are perfect example of this... You even put into comments what should we wait for.
This is the strongest reason preventing me from giving approval to this PR. If it works OK without them (even on CI), we could remove them. If it doesn't work right without them, we should replace them with conditional waits.

await this.driverHelper.waitAndClick(configurationItemLocator);

// for ensure that item is selected
await this.driverHelper.wait(1000);
await this.ide.performKeyCombination(Key.ESCAPE);

// for ensure that drop-down list has been closed
await this.driverHelper.wait(2000);
}

async clickOnRunDebugButton() {
const runDebugButtonLocator: By = By.xpath('//span[@title=\'Start Debugging\']');

await this.driverHelper.waitAndClick(runDebugButtonLocator);
}

}
Loading