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

Proof of Concept - Playwright #571

Merged
merged 30 commits into from
Aug 3, 2022

Conversation

cbeach47
Copy link
Contributor

@cbeach47 cbeach47 commented Jul 19, 2022

Checklist
Description of change

Proof of concept for Playwright test harness integration into uPortal-start.

Initial round of tests focus on an upcoming feature in uPortal, the portal-list api.

Near team goals:

  • Align on overall Playwright integration Gradle tasks / flow
  • Ensure tests are repeatable and can be run in parallel
  • Install Playwright and run tests as part of the PR and main branch CI flows

@cbeach47 cbeach47 changed the title Testing portlet lists api Proof of Concept - Playwright Jul 19, 2022
gradle/tasks/playwright.gradle Outdated Show resolved Hide resolved
gradle/tasks/playwright.gradle Outdated Show resolved Hide resolved
gradle/tasks/playwright.gradle Outdated Show resolved Hide resolved
tests/api/utils/api-portlet-list-utils.ts Outdated Show resolved Hide resolved
tests/api/utils/api-portlet-list-utils.ts Outdated Show resolved Hide resolved
tests/api/utils/api-portlet-list-utils.ts Outdated Show resolved Hide resolved
tests/ux/utils/ux-general-utils.ts Outdated Show resolved Hide resolved
tests/ux/utils/ux-general-utils.ts Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
@ChristianMurphy
Copy link
Member

Error showing in CI right now is also happening on master, and appears to be unrelated to these changes

Caused by: org.gradle.api.UncheckedIOException: java.io.IOException: Unable to delete file 'D:\a\uPortal-start\uPortal-start\.gradle\tomcat\logs\catalina.2022-06-13.log'

https://github.com/uPortal-Project/uPortal-start/runs/6866142583?check_suite_focus=true#step:8:381

Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

Is this at a point where we could add part (or all of this) to continuous integration (GitHub actions)?

README.md Outdated Show resolved Hide resolved
@@ -1,5 +1,9 @@
apply plugin: 'com.github.node-gradle.node'

node {
download = true
Copy link
Member

Choose a reason for hiding this comment

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

do we also want/need to declare the version of node, like:

node {
version = nodejsVersion
download = true
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

need? no (at least not yet). When we bump up to the next version of the node plugin, we may want to control the version.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the node versions between:
uPortal-start's current

nodejsVersion=12.16.1

and the gradle plugin are several major apart https://github.com/node-gradle/gradle-node-plugin/blob/1330dc94ce8cc5b2a64d77e4fd38c0792946378d/src/main/kotlin/com/github/gradle/node/NodeExtension.kt#L134

I do think uPortal-start's probably can be upgraded to 16, and this may be fine for now.
Just something to be aware of.

@cbeach47
Copy link
Contributor Author

Is this at a point where we could add part (or all of this) to continuous integration (GitHub actions)?

Not yet, since the portlet-list API has not yet been merged into the main branch of uPortal.

@ChristianMurphy
Copy link
Member

Not yet, since the portlet-list API has not yet been merged into the main branch of uPortal.

The tsc, type-coverage, and eslint steps are generic enough for that to not matter? 🤔
But I take your point that we can't run playwright itself on the (unmerged) API yet.

@cbeach47
Copy link
Contributor Author

Not yet, since the portlet-list API has not yet been merged into the main branch of uPortal.

The tsc, type-coverage, and eslint steps are generic enough for that to not matter? 🤔 But I take your point that we can't run playwright itself on the (unmerged) API yet.

Good point - Setting up the test harness has nothing to do with the unmerged APIs, so that's not a blocker.

We can also stand up a quick login / logout test so something is run.

@cbeach47
Copy link
Contributor Author

cbeach47 commented Aug 2, 2022

This PR no longer is dependent on the portlet-lists api. It has a small number of page (ux) and api tests.

Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

Could we include some of these tests as part of CI/CD?
Somewhere in/around

- name: Build
run: ./gradlew -i -S build
- name: Tomcat Install
run: ./gradlew -i -S tomcatInstall
- name: Start Tomcat
run: ./gradlew -i -S tomcatStart
- name: Stop Tomcat
run: ./gradlew -i -S tomcatStop
- name: Clear Logs Tomcat
run: ./gradlew -i -S tomcatClearLogs
- name: Start Database
run: ./gradlew -i -S hsqlStart
- name: Init Data
run: ./gradlew -i -S dataInit
- name: Stop Database
run: ./gradlew -i -S hsqlStop
- name: Generate Skin
run: ./gradlew -i -S skinGenerate -DskinName=test

Also could we look into the merge conflict?

@cbeach47
Copy link
Contributor Author

cbeach47 commented Aug 2, 2022

merge conflict is now fixed.

I'll add playwrightRun into the CI flow.

@cbeach47
Copy link
Contributor Author

cbeach47 commented Aug 2, 2022

hmm the tests are not finding the expected data. taking a look...

@cbeach47
Copy link
Contributor Author

cbeach47 commented Aug 3, 2022

Setting aside the effort to include playwright in the CI flow for now. Tracked via #574 .

Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @cbeach47!

tests/ux/utils/ux-general-utils.ts Outdated Show resolved Hide resolved
@cbeach47 cbeach47 merged commit 9592f66 into uPortal-Project:master Aug 3, 2022
@cbeach47 cbeach47 deleted the testing-portlet-lists-api branch August 3, 2022 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants