WIP: Feature implement python devfile #15736

ScrewTSW · 2020-01-17T11:14:56Z

What does this PR do?

fixes Ide.closeAllNotifications method which was not functional, replacing the logic with use of NotificationCenter instead
adds a new method Ide.waitDialogShell into the Ide class which allows handling of the full-screen notification overlays
adds runTaskWithDialogShell method into the common class which handles tasks that don't have an exit code toast displayed after they're executed (namely webserver containing devfiles)
adds a new Python Django devfile test
fixes some small typos

What issues does this PR fix or reference?

#15735

che-bot · 2020-01-17T11:14:58Z

Can one of the admins verify this patch?

che-bot · 2020-01-17T11:14:58Z

Can one of the admins verify this patch?

che-bot · 2020-01-17T11:39:04Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

rhopp · 2020-01-17T11:40:22Z

ci-test

che-bot · 2020-01-17T11:45:53Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

che-bot · 2020-01-17T11:49:50Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

che-bot · 2020-01-17T11:50:58Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

Ohrimenko1988

Please rewrite your test methods by using the "DriverHelper" class.

Katka92

Adding the fist part of the review. I'll continue on Monday.

tests/e2e/mocha-python-django.opts

tests/e2e/tests/devfiles/PythonDjango.spec.ts

tests/e2e/pageobjects/ide/Ide.ts

che-bot · 2020-01-20T09:25:53Z

E2E tests of Eclipse Che Multiuser on OCP has failed:

Build details
Test report
docker image: eclipseche/che-server:15736
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.
Use command "ci-test" to rerun the test.

Katka92

Maybe it would be good trying to simplify locators using this approach: https://github.com/eclipse/che/blob/master/tests/e2e/pageobjects/ide/DebugView.ts#L33 - I mean using contains for some important class instead of directly specifying all classes by @class.

tests/e2e/testsLibrary/CodeExecutionTests.ts

tests/e2e/pageobjects/ide/Ide.ts

che-bot · 2020-01-21T15:36:47Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

ScrewTSW · 2020-01-22T08:41:06Z

tests/e2e/pageobjects/ide/Ide.ts

-        timeout: number = TestConstants.TS_SELENIUM_LOAD_PAGE_TIMEOUT) {
-
+    async waitWorkspaceAndIde(workspaceNamespace: string, workspaceName: string, timeout: number = TestConstants.TS_SELENIUM_LOAD_PAGE_TIMEOUT) {
+        // why are there unused arguments in this method?


We should consider refactoring this method. It's tied to HappyPath tests as well.

@Ohrimenko1988 ^

che-bot · 2020-01-22T09:12:08Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

tests/e2e/pageobjects/ide/Ide.ts

che-bot · 2020-01-22T16:49:14Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

che-bot · 2020-01-23T15:51:11Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

che-bot · 2020-01-23T16:57:33Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

tests/e2e/pageobjects/ide/Ide.ts

Ohrimenko1988 · 2020-01-24T13:17:43Z

tests/e2e/pageobjects/ide/TopMenu.ts

@@ -45,8 +43,6 @@ export class TopMenu {
        Logger.debug(`TopMenu.clickOnTopMenuButton "${buttonText}"`);

        const buttonLocator: By = this.getTopMenuButtonLocator(buttonText);
-
-        await this.ide.closeAllNotifications();


Could you explain a reason why you deleted this method

It makes absolutely no sense to close notifications after clicking the top menu item and it was stealing focus from the Terminal element causing the windows to close since the method closeAllNotifications is performing clicks.

If you want to make sure there are no notifications present, you should call it in your tests separately before you call TopMenu.clickOnTopMenuButton

I've discussed it with @Katka92 and @rhopp and they agreed

It doesn't make sense if notifications appear at the bottom left corner. as now But this method was put there when notifications appeared at the top center and override the top menu. Can you guarantee that it will not be there again for example tomorrow?

Ohrimenko1988 · 2020-01-24T13:22:00Z

tests/e2e/testsLibrary/CodeExecutionTests.ts

+export function runTaskWithDialogShellAndOpenLink(taskName: string, expectedDialogText: string, timeout: number) {
+    test(`Run command '${taskName}' expecting dialog shell`, async () => {
+        await topMenu.runTask(taskName);
+        await dialogWindow.waitDialog(timeout, expectedDialogText);


replace

await dialogWindow.waitDialog(timeout, expectedDialogText); await dialogWindow.clickToButton(dialogWindowOpenLinkButtonText); await dialogWindow.waitDialogDissappearance();

by

await dialogWindow.waitDialogAndOpenLink(timeout, expectedDialogText);

https://github.com/eclipse/che/blob/master/tests/e2e/pageobjects/ide/DialogWindow.ts#L79 this and the method above it seem to be in an incorrect order

I've fixed the method:
From:

await this.ide.waitApllicationIsReady(await this.getApplicationUrlFromDialog(dialogText), timeout); await this.clickToOpenLinkButton();

To:

const applicationUrlFromDialog: string = await this.getApplicationUrlFromDialog(dialogText); await this.clickToOpenLinkButton(); await this.ide.waitApllicationIsReady(applicationUrlFromDialog, timeout);

This resolved the race condition

… added new runTaskWithDialogShell method into testsLibrary/CodeExecutionTests Signed-off-by: Tibor Dancs <tdancs@redhat.com>

Signed-off-by: Tibor Dancs <tdancs@redhat.com>

che-bot · 2020-01-24T13:52:08Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

che-bot · 2020-01-24T14:16:56Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15736

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

ScrewTSW · 2020-01-27T10:03:45Z

Closing. The PR was split into two separate PRs and cross-referenced:
#15830
#15831

Ohrimenko1988 · 2020-01-24T13:35:54Z

tests/e2e/utils/DriverHelper.ts

@@ -521,6 +522,32 @@ export class DriverHelper {
        throw new error.TimeoutError(`Exceeded maximum mouse move attempts, for the '${elementLocator}' element`);
    }

+    public async hasChildren(elementLocator: By): Promise<boolean> {


I can suggest other variant:

public async hasChildren(elementXpathLocator: String): Promise<boolean> { return await this.isVisible(By.xpath(`${elementXpathLocator}//*`)); }

For me looks much simplier

Ohrimenko1988 · 2020-01-24T13:41:08Z

tests/e2e/pageobjects/ide/TopMenu.ts

@@ -45,8 +43,6 @@ export class TopMenu {
        Logger.debug(`TopMenu.clickOnTopMenuButton "${buttonText}"`);

        const buttonLocator: By = this.getTopMenuButtonLocator(buttonText);
-
-        await this.ide.closeAllNotifications();


It doesn't make sense if notifications appear at the bottom left corner. as now But this method was put there when notifications appeared at the top center and override the top menu. Can you guarantee that it will not be there again for example tomorrow?

tests/e2e/pageobjects/ide/Ide.ts

ScrewTSW requested review from l0rd, nickboldt, rhopp and vparfonov as code owners January 17, 2020 11:14

che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Jan 17, 2020

ScrewTSW force-pushed the feature-implement-python-devfile branch 3 times, most recently from c4a8020 to 76c0fee Compare January 17, 2020 11:33

Ohrimenko1988 requested changes Jan 17, 2020

View reviewed changes

Katka92 requested changes Jan 17, 2020

View reviewed changes

tests/e2e/mocha-python-django.opts Show resolved Hide resolved

tests/e2e/tests/devfiles/PythonDjango.spec.ts Outdated Show resolved Hide resolved

tests/e2e/pageobjects/ide/Ide.ts Outdated Show resolved Hide resolved

tests/e2e/pageobjects/ide/Ide.ts Outdated Show resolved Hide resolved

Katka92 reviewed Jan 20, 2020

View reviewed changes

tests/e2e/testsLibrary/CodeExecutionTests.ts Outdated Show resolved Hide resolved

tests/e2e/pageobjects/ide/Ide.ts Outdated Show resolved Hide resolved

tests/e2e/pageobjects/ide/Ide.ts Outdated Show resolved Hide resolved

ScrewTSW force-pushed the feature-implement-python-devfile branch 2 times, most recently from 63079ff to 6361103 Compare January 21, 2020 15:13

ScrewTSW commented Jan 22, 2020

View reviewed changes

ScrewTSW force-pushed the feature-implement-python-devfile branch from 6361103 to 1e8e2e0 Compare January 22, 2020 08:46

Ohrimenko1988 reviewed Jan 22, 2020

View reviewed changes

tests/e2e/pageobjects/ide/Ide.ts Outdated Show resolved Hide resolved

Ohrimenko1988 reviewed Jan 22, 2020

View reviewed changes

tests/e2e/pageobjects/ide/Ide.ts Outdated Show resolved Hide resolved

Ohrimenko1988 reviewed Jan 22, 2020

View reviewed changes

tests/e2e/pageobjects/ide/Ide.ts Show resolved Hide resolved

ScrewTSW force-pushed the feature-implement-python-devfile branch from 1e8e2e0 to ec051a0 Compare January 22, 2020 14:58

ScrewTSW force-pushed the feature-implement-python-devfile branch 2 times, most recently from 20e279e to 3c7a54d Compare January 22, 2020 15:08

eclipse-che deleted a comment from che-bot Jan 22, 2020

ScrewTSW force-pushed the feature-implement-python-devfile branch from 3c7a54d to dcf6901 Compare January 23, 2020 15:27

ScrewTSW force-pushed the feature-implement-python-devfile branch from dcf6901 to 920a177 Compare January 23, 2020 16:29

Katka92 approved these changes Jan 24, 2020

View reviewed changes

ScrewTSW requested a review from Ohrimenko1988 January 24, 2020 09:34

Ohrimenko1988 reviewed Jan 24, 2020

View reviewed changes

tests/e2e/pageobjects/ide/Ide.ts Show resolved Hide resolved

Ohrimenko1988 reviewed Jan 24, 2020

View reviewed changes

tests/e2e/pageobjects/ide/Ide.ts Outdated Show resolved Hide resolved

Ohrimenko1988 reviewed Jan 24, 2020

View reviewed changes

ScrewTSW force-pushed the feature-implement-python-devfile branch from 920a177 to 8dfbb3d Compare January 24, 2020 13:25

ScrewTSW added 2 commits January 24, 2020 14:48

Fixed closeAllNotifications method, added new waitDialogShell method,…

21e1296

… added new runTaskWithDialogShell method into testsLibrary/CodeExecutionTests Signed-off-by: Tibor Dancs <tdancs@redhat.com>

Implemented new Python Django devfile test

44e9f6c

Signed-off-by: Tibor Dancs <tdancs@redhat.com>

ScrewTSW force-pushed the feature-implement-python-devfile branch from 8dfbb3d to 44e9f6c Compare January 24, 2020 13:48

ScrewTSW changed the title ~~Feature implement python devfile~~ WIP: Feature implement python devfile Jan 24, 2020

This was referenced Jan 27, 2020

Refactor upstream tests to allow easier devfile implementation #15830

Merged

Implement Python Django devfile test #15831

Merged

ScrewTSW closed this Jan 27, 2020

Ohrimenko1988 reviewed Jan 28, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Feature implement python devfile #15736

WIP: Feature implement python devfile #15736

ScrewTSW commented Jan 17, 2020

che-bot commented Jan 17, 2020

che-bot commented Jan 17, 2020

che-bot commented Jan 17, 2020

rhopp commented Jan 17, 2020

che-bot commented Jan 17, 2020

che-bot commented Jan 17, 2020

che-bot commented Jan 17, 2020

Ohrimenko1988 left a comment

Katka92 left a comment

che-bot commented Jan 20, 2020

Katka92 left a comment

che-bot commented Jan 21, 2020

ScrewTSW Jan 22, 2020

ScrewTSW Jan 24, 2020

che-bot commented Jan 22, 2020

che-bot commented Jan 22, 2020

che-bot commented Jan 23, 2020

che-bot commented Jan 23, 2020

Ohrimenko1988 Jan 24, 2020

ScrewTSW Jan 24, 2020 •

edited

Loading

Ohrimenko1988 Jan 24, 2020

Ohrimenko1988 Jan 24, 2020

ScrewTSW Jan 24, 2020

ScrewTSW Jan 24, 2020

che-bot commented Jan 24, 2020

che-bot commented Jan 24, 2020

ScrewTSW commented Jan 27, 2020

Ohrimenko1988 Jan 24, 2020

Ohrimenko1988 Jan 24, 2020

WIP: Feature implement python devfile #15736

WIP: Feature implement python devfile #15736

Conversation

ScrewTSW commented Jan 17, 2020

What does this PR do?

What issues does this PR fix or reference?

che-bot commented Jan 17, 2020

che-bot commented Jan 17, 2020

che-bot commented Jan 17, 2020

rhopp commented Jan 17, 2020

che-bot commented Jan 17, 2020

che-bot commented Jan 17, 2020

che-bot commented Jan 17, 2020

Ohrimenko1988 left a comment

Choose a reason for hiding this comment

Katka92 left a comment

Choose a reason for hiding this comment

che-bot commented Jan 20, 2020

Katka92 left a comment

Choose a reason for hiding this comment

che-bot commented Jan 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

che-bot commented Jan 22, 2020

che-bot commented Jan 22, 2020

che-bot commented Jan 23, 2020

che-bot commented Jan 23, 2020

Choose a reason for hiding this comment

ScrewTSW Jan 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

che-bot commented Jan 24, 2020

che-bot commented Jan 24, 2020

ScrewTSW commented Jan 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ScrewTSW Jan 24, 2020 •

edited

Loading