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

Move Dev Spaces typescript functional tests, clean up outdated code #22100

Merged
merged 4 commits into from
Mar 29, 2023

Conversation

nallikaea
Copy link
Contributor

@nallikaea nallikaea commented Mar 27, 2023

What does this PR do?

  • Move Factory tests and Quarkus sample test from downstream to this project;
  • Created logout() method to make possible to run a few test specs in one test run;
  • README updated;
  • Outdated code deleted;
  • Some typos fixed;

Screenshot/screencast of this PR

image

Full log:
test_output_log.txt

None: some tests skipped because of specific cluster set ups.

What issues does this PR fix or reference?

CRW-4091

How to test this PR?

export USERSTORY=Quarkus && npm run test-userstory

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. target/branch Indicates that a PR will be merged into a branch other than master. labels Mar 27, 2023
----------------------------------------------*/
TS_SELENIUM_TYPESCRIPT_DEBUG_IMAGE: process.env.TS_SELENIUM_TYPESCRIPT_DEBUG_IMAGE || 'quay.io/eclipse/che-nodejs10-ubi:7.35.2',

TS_SELENIUM_PHP_DEBUG_IMAGE: process.env.TS_SELENIUM_PHP_DEBUG_IMAGE || 'registry.redhat.io/codeready-workspaces/stacks-php-rhel8:latest',
Copy link
Contributor

@dmytro-ndp dmytro-ndp Mar 27, 2023

Choose a reason for hiding this comment

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

It could be probably outdated.

Would you revise constants and remove those which havn't been used in tests after clean up of Che Theia tests?

Copy link
Contributor Author

@nallikaea nallikaea Mar 27, 2023

Choose a reason for hiding this comment

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

If there no any reference in tests, should I delete it? Nothing uses for example on ci somehow?

Copy link
Contributor

@dmytro-ndp dmytro-ndp Mar 27, 2023

Choose a reason for hiding this comment

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

Yes, there are no other downstream tests but e2e-codeready project.

Upstream CIs are:

e2eContainer.bind<Sanitizer>(CLASSES.Sanitizer).to(Sanitizer);
e2eContainer.bind<ApiUrlResolver>(CLASSES.ApiUrlResolver).to(ApiUrlResolver);
e2eContainer.bind<WorkspaceHandlingTests>(CLASSES.WorkspaceHandlingTests).to(WorkspaceHandlingTests);

if (TestConstants.TS_OCP_LOGIN_PAGE_PROVIDER_TITLE === 'DevSandbox') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be simpler to get rid of ICheLoginPage and use RedHatLoginPage only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be, but can you help me with the list of instances I should test that loggin is correct?
cc: @ScrewTSW

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I know, these tests are run against OCP only, including:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ScrewTSW could you help me to run tests on DevSandbox and dogfooding locally, please?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you, done

Copy link
Contributor

@dmytro-ndp dmytro-ndp Mar 29, 2023

Choose a reason for hiding this comment

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

Test build failed:

                            docker run --shm-size=${shmSize} \\
                            -v ${WORKSPACE}/report/${USERNAME}:/tmp/report:Z \\
                            -v ${WORKSPACE}/video/${USERNAME}:/tmp/ffmpeg_report:Z \\
                            -e TS_SELENIUM_BASE_URL="${accountDsDashboardUrl}" \\
                            -e TS_SELENIUM_LOG_LEVEL="DEBUG" \\
                            -e TS_SELENIUM_USERNAME=${USERNAME} \\
                            -e TS_SELENIUM_PASSWORD=${PASSWORD} \\
                            -e E2E_OPENSHIFT_TOKEN="${accountToken}" \\
                            -e DELETE_WORKSPACE_ON_FAILED_TEST=true \\
                            -e TS_SELENIUM_START_WORKSPACE_TIMEOUT=${START_WORKSPACE_TIMEOUT} \\
                            -e NODE_TLS_REJECT_UNAUTHORIZED=0 \\
                            -e TS_SELENIUM_EDITOR=che-code \\
                            -e USERSTORY=EmptyWorkspace \\
                            -e TS_OCP_LOGIN_PAGE_PROVIDER_TITLE="DevSandbox" \\
                            -e TS_SELENIUM_REPORT_FOLDER="/tmp/report" \\
                            -e VIDEO_RECORDING=true \\
                            quay.io/crw_pr/che-e2e:22100

...

 Running TEST_SUITE: test-happy-path with user: osio-ci-devsandbox-periodic-stg
10:00:34  + ffmpeg_pid=127
10:00:34  + trap kill_ffmpeg 2 15
10:00:34  + echo 'Running TEST_SUITE: test-happy-path with user: osio-ci-devsandbox-periodic-stg'
10:00:34  + npm run test-happy-path
10:00:34  + nohup ffmpeg -y -video_size 1920x1080 -framerate 24 -f x11grab -i :20.0 /tmp/ffmpeg_report/output.mp4
10:00:35  npm ERR! Missing script: "test-happy-path"
...

https://main-jenkins-csb-crwqe.apps.ocp-c1.prod.psi.redhat.com/job/DevSandbox/job/sandbox-test/5711/console

Copy link
Contributor

@dmytro-ndp dmytro-ndp Mar 29, 2023

Choose a reason for hiding this comment

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

For the record: EmptyWorkspace test passed being run against Dev Spaces on regular OCP cluster https://main-jenkins-csb-crwqe.apps.ocp-c1.prod.psi.redhat.com/job/Testing/job/e2e/job/basic/job/typescript-tests/7293/

docker run --shm-size=4096m -p 5920:5920 \ 
-e TS_SELENIUM_LOAD_PAGE_TIMEOUT=420000 \ 
-e TS_SELENIUM_BASE_URL=https://devspaces.apps.ocp412.crw-qe.com/ \ 
-e DELETE_WORKSPACE_ON_FAILED_TEST=true \ 
-e TS_SELENIUM_START_WORKSPACE_TIMEOUT=600000 \ 
-e NODE_TLS_REJECT_UNAUTHORIZED=0 \ 
-e VIDEO_RECORDING=true \ 
-e E2E_OCP_CLUSTER_API_URL=https://api.ocp412.crw-qe.com:6443/ \ 
-e TS_SELENIUM_LOG_LEVEL=TRACE \ 
-e TS_EDITOR_TAB_INTERACTION_TIMEOUT=20_000 \ 
-e TS_WAIT_LOADER_PRESENCE_TIMEOUT=600000 \ 
-e OCP_INFRA=PSI \
-v /tmp:/usr/local/bin:Z \ 
-v /home/hudson/.kube:/home/seluser/.kube:Z \ 
-v /mnt/hudson_workspace/workspace/Testing/e2e/basic/typescript-tests/test-repo/tests/e2ereport:/tmp/e2e/report:Z \ 
-v /mnt/hudson_workspace/workspace/Testing/e2e/basic/typescript-tests/test-repo/tests/e2evideo:/tmp/ffmpeg_report:Z \ 
-e USERSTORY=EmptyWorkspace \ 
-e TS_SELENIUM_VALUE_OPENSHIFT_OAUTH=true \ 
-e TS_OCP_LOGIN_PAGE_PROVIDER_TITLE=htpasswd \ 
-e TS_SELENIUM_OCP_USERNAME=***** \ 
-e 'TS_SELENIUM_OCP_PASSWORD=******' \ 
-e TS_SELENIUM_EDITOR=che-code quay.io/crw_pr/che-e2e:22100

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@dmytro-ndp
Copy link
Contributor

@nallikaea : what do you think about further code simplification, and getting rid of inversify library, together with interfaces:

  • replace IOcpLoginPage with RedHatLoginPage
  • replace IDriver with ChromeDriver
  • replace ITestWorkspaceUtil with TestWorkspaceUtil
  • replace ICheLoginPage with RedHatLoginPage
  • delete IAuthorizationHeaderHandler and CheMultiuserAuthorizationHeaderHandler
    ?

@nallikaea
Copy link
Contributor Author

@nallikaea : what do you think about further code simplification, and getting rid of inversify library, together with interfaces:

  • replace IOcpLoginPage with RedHatLoginPage
  • replace IDriver with ChromeDriver
  • replace ITestWorkspaceUtil with TestWorkspaceUtil
  • replace ICheLoginPage with RedHatLoginPage
  • delete IAuthorizationHeaderHandler and CheMultiuserAuthorizationHeaderHandler
    ?

We can do it
Maybe it is better to create another PR?

Copy link
Contributor

@artaleks9 artaleks9 left a comment

Choose a reason for hiding this comment

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

It seems, this can be deleted from the 'TestConstans':
TS_SELENIUM_SINGLE_HOST
TS_SELENIUM_USERNAME
TS_SELENIUM_PASSWORD
TS_SELENIUM_EMAIL_USER
TS_SELENIUM_FIRST_NAME
TS_SELENIUM_LAST_NAME

Is this used anywhere?
@dmytro-ndp @ScrewTSW

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Mar 27, 2023

@nallikaea : what do you think about further code simplification, and getting rid of inversify library, together with interfaces:

  • replace IOcpLoginPage with RedHatLoginPage
  • replace IDriver with ChromeDriver
  • replace ITestWorkspaceUtil with TestWorkspaceUtil
  • replace ICheLoginPage with RedHatLoginPage
  • delete IAuthorizationHeaderHandler and CheMultiuserAuthorizationHeaderHandler
    ?

We can do it Maybe it is better to create another PR?

IMHO, it should make migration simpler, shouldn't it?
Why not do it right now, all at once?

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Mar 27, 2023

It seems, this can be deleted from the 'TestConstans': TS_SELENIUM_SINGLE_HOST TS_SELENIUM_USERNAME TS_SELENIUM_PASSWORD TS_SELENIUM_EMAIL_USER TS_SELENIUM_FIRST_NAME TS_SELENIUM_LAST_NAME

Is this used anywhere? @dmytro-ndp @ScrewTSW

Agree.
It looks like leftovers from the time we had been testing Che Keycloak.

@ScrewTSW
Copy link
Member

@dmytro-ndp @nallikaea AFAIK TS_SELENIUM_USERNAME and TS_SELENIUM_PASSWORD are still utilized in various login providers. I'd be careful about those two.

@nallikaea
Copy link
Contributor Author

nallikaea commented Mar 27, 2023

TS_SELENIUM_PASSWORD and TS_SELENIUM_USERNAME used in the classes RedHatLoginPage and OcpRedHatLoginPage.

@injectable()
export class RedHatLoginPage {

    constructor(
        @inject(CLASSES.DriverHelper) private readonly driverHelper: DriverHelper) { }

    async waitRedHatLoginWelcomePage() {
        Logger.debug('RedHatLoginPage.waitRedHatLoginWelcomePage');
        await this.driverHelper.waitVisibility(By.id(USERNAME_INPUT_ID));
    }

    async enterPasswordRedHat() {
        Logger.debug('RedHatLoginPage.enterPasswordRedHat');
        const passwordFieldLocator: By = By.id(PASSWORD_INPUT_ID);
        await this.driverHelper.waitVisibility(passwordFieldLocator, 3000);
        await this.driverHelper.enterValue(passwordFieldLocator, **TestConstants.TS_SELENIUM_PASSWORD,** timeout );
if (TestConstants.TS_OCP_LOGIN_PAGE_PROVIDER_TITLE === 'DevSandbox') {
    e2eContainer.bind<RedHatLoginPage>(CLASSES.RedHatLoginPage).to(RedHatLoginPage);
    e2eContainer.rebind<ICheLoginPage>(TYPES.CheLogin).to(OcpRedHatLoginPage);
}

@ScrewTSW @dmytro-ndp @artaleks9 so should we keep it or it outdated pageoblects?

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Mar 27, 2023

@nallikaea , @ScrewTSW, @artaleks9 : okay, I have also found https://gitlab.cee.redhat.com/codeready-workspaces/crw-jenkins/-/blob/master/jobs/DevSandbox/sandbox-test.jenkinsfile#L175-177

Let's keep next constants then:

  • TS_SELENIUM_PASSWORD
  • TS_SELENIUM_USERNAME

@nallikaea
Copy link
Contributor Author

nallikaea commented Mar 27, 2023

I updated PR

  9 passing (2m)

            ‣ DriverHelper.wait (5000 milliseconds)
            ‣ DriverHelper.getDriver

Process finished with exit code 0

tests/e2e/package.json Outdated Show resolved Hide resolved
@nallikaea nallikaea force-pushed the crw-4091 branch 4 times, most recently from 86cecc2 to ccf317e Compare March 28, 2023 14:35
Signed-off-by: mdolhalo <mdolhalo@redhat.com>
mdolhalo added 2 commits March 29, 2023 10:24
Signed-off-by: mdolhalo <mdolhalo@redhat.com>
Signed-off-by: mdolhalo <mdolhalo@redhat.com>
Signed-off-by: mdolhalo <mdolhalo@redhat.com>

Signed-off-by: mdolhalo <mdolhalo@redhat.com>
Copy link
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

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

Looks good to merge.

Thanks for handling feedback, @nallikaea !

Copy link
Member

@ScrewTSW ScrewTSW left a comment

Choose a reason for hiding this comment

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

LGTM

@dmytro-ndp dmytro-ndp merged commit db63138 into eclipse-che:main Mar 29, 2023
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Mar 29, 2023
@nallikaea nallikaea deleted the crw-4091 branch April 13, 2023 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
target/branch Indicates that a PR will be merged into a branch other than master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants