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.

Sign up for GitHub

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

Jump to bottom

Added Eclipse Che typescript E2E functional tests PR check #22377

Merged
merged 1 commit into from
Jul 25, 2023
Merged

Added Eclipse Che typescript E2E functional tests PR check #22377

merged 1 commit into from
Jul 25, 2023

Conversation

nallikaea
Copy link
Contributor

@nallikaea nallikaea commented Jul 19, 2023 •
edited
Loading

What does this PR do?

Resolve issue #22044

  • Added GitHub Action that runs Empty Workspace API test, build Che e2e test container and runs Empty Workspace UI test;
  • Empty Workspace tests were choosen because of low resource of VM (7RAM, 10Gb disk), other cases returned '0/1 nodes are available: 1 Insufficient cpu. preemption: 0/1 nodes are available: 1 No preemption victims found for incoming pod..'
  • Updated Dashboard pageobject according to Standardization of "che" test automation framework classes #22184
  • Updated quarkus, smoke and empty ws tests to improve stability and avoid hard-coded values
  • Added possibility to work with factory urls with query parameters (like 'https://github.com/quarkus-api-example?df=smoke-test.devfile.yaml') into method getProjectNameFromGitUrl()
  • Renamed other actions with understandable names

Screenshot/screencast of this PR

image
image

What issues does this PR fix or reference?

Resolve issue #22044

How to test this PR?

https://github.com/eclipse/che/actions/runs/5629951250

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 Jul 19, 2023
@nallikaea nallikaea force-pushed the 22044 branch 13 times, most recently from c108674 to d5e4a86 Compare July 21, 2023 21:03
@nallikaea nallikaea self-assigned this Jul 22, 2023
@nallikaea nallikaea linked an issue Jul 22, 2023 that may be closed by this pull request
Restore Eclipse Che typescript E2E functional tests PR check #22044
Closed
@nallikaea nallikaea force-pushed the 22044 branch 2 times, most recently from 06cb615 to e3db5f6 Compare July 22, 2023 09:26
@nallikaea nallikaea changed the title WIP: 22044 Added Eclipse Che typescript E2E functional tests PR check Jul 22, 2023
@che-bot che-bot added the kind/task Internal things, technical debt, and to-do tasks to be performed. label Jul 24, 2023
ScrewTSW
ScrewTSW approved these changes Jul 25, 2023
View reviewed changes
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.

Do you have any example run of SmokeTest suite?

dmytro-ndp
dmytro-ndp reviewed Jul 25, 2023
View reviewed changes
.github/workflows/next-build.yml Outdated
@@ -7,7 +7,7 @@
# SPDX-License-Identifier: EPL-2.0
#

name: build-next
name: Build and push Next Che E2E image to quai.io
Copy link
Contributor

Choose a reason for hiding this comment

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

build-next is unified name across Eclipse Che component repos, e.g. https://github.com/eclipse-che/che-server/blob/main/.github/workflows/next-build.yml

So, I would keep it untouched.

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 is just a name which will be shown in Actions tab and has no effect on build actions:

https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#name

I believe it should be clear what has built, why and where was pushed from this name. You don`t need to go through all workflow code to understand that.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I see.

My point was that the this eclipse/che repo is not dedicated to e2e tests (looking at the root folder files), so I was not sure if it's eligible to modify build-next action.

Moreover - this action has outdated code which we don't need anymore:

    - name: Cache local Maven repository
      uses: actions/cache@v3
      with:
       path: ~/.m2/repository
       key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
       restore-keys: |
        ${{ runner.os }}-maven-      
    - name: Set up JDK 11
      uses: actions/setup-java@v3
      with:
        distribution: 'adopt'
        java-version: '11'

Anyway, we can consider movement to dedicated repo https://github.com/eclipse-che/che-tests

BTW: there is a TYPO in name: quai.io > quay.io.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@nallikaea
Copy link
Contributor Author

nallikaea commented Jul 25, 2023 •
edited
Loading

LGTM.

Do you have any example run of SmokeTest suite?

yes: https://github.com/eclipse/che/actions/runs/5624111804

as I wrote in description I got the error '0/1 nodes are available: 1 Insufficient cpu. preemption: 0/1 nodes are available: 1 No preemption victims found for incoming pod..', you can check logs in artifacts. It because low resources on VM (said insufficient cpu)

dmytro-ndp
dmytro-ndp reviewed Jul 25, 2023
View reviewed changes
.github/workflows/pr-check.yml
- name: Deploy Che
run: |
#
# load Che-Code image into minikube
Copy link
Contributor

@dmytro-ndp dmytro-ndp Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It has to be next version for Eclipse Che Next (--channel=next in line 57 above).

Therefore, correct image is address: quay.io/che-incubator/che-code:next .

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 tested that test commands still works correctly, not our product. I think we can use stable version here.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's about checking PRs into the main branch, isn't it?
And main branch is dedicated to testing Eclipse Che Next.

So, it's reasonable to run PR check against Eclipse Che Next.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

dmytro-ndp
dmytro-ndp reviewed Jul 25, 2023
View reviewed changes
.github/workflows/pr-check.yml
cd tests/e2e
docker build -t quay.io/eclipse/che-e2e:"${{ env.pr_number }}" -f build/dockerfiles/Dockerfile .

- name: Run Empty Workspace UI test from che-e2e container
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO, it make sense to run SmokeTest instead - it covers project import additionally https://github.com/eclipse/che/blob/main/tests/e2e/specs/SmokeTest.spec.ts

Also, there is a plan to run SmokeTest in PR check of Eclipse Che repos:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please, read my description

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 check that changes not broke e2e test functionality, commands from packege.json. We don`t check "che" here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#22377 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's comment it somewhere in the repo what exactly does PR check do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

dmytro-ndp
dmytro-ndp approved these changes Jul 25, 2023
View reviewed changes
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 commonly good with a few non-critical notices.

@nallikaea nallikaea force-pushed the 22044 branch 2 times, most recently from da9e622 to d6f700d Compare July 25, 2023 09:57
feat: PR check GitHub Action
9a47941 
Signed-off-by: mdolhalo <mdolhalo@redhat.com>
@nallikaea nallikaea merged commit cc6cd70 into eclipse-che:main Jul 25, 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 Jul 25, 2023
@nallikaea nallikaea deleted the 22044 branch August 24, 2023 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmytro-ndp dmytro-ndp dmytro-ndp approved these changes

@ScrewTSW ScrewTSW ScrewTSW approved these changes

@musienko-maxim musienko-maxim musienko-maxim approved these changes

@nickboldt nickboldt Awaiting requested review from nickboldt

Assignees

@nallikaea nallikaea

Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. target/branch Indicates that a PR will be merged into a branch other than master.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Restore Eclipse Che typescript E2E functional tests PR check
5 participants
@nallikaea @dmytro-ndp @musienko-maxim @ScrewTSW @che-bot