Skip to content

fix e2e tests for helix-front#2144

Merged
junkaixue merged 34 commits intoapache:masterfrom
micahstubbs:2075/e2e-tests-chrome-issue
Jun 15, 2022
Merged

fix e2e tests for helix-front#2144
junkaixue merged 34 commits intoapache:masterfrom
micahstubbs:2075/e2e-tests-chrome-issue

Conversation

@micahstubbs
Copy link
Contributor

@micahstubbs micahstubbs commented Jun 7, 2022

Issues

  • My PR addresses the following Helix issues and references them in the PR description:

fix #2075 #2148

Description

  • Here are some details about my PR, including screenshots of any UI changes:

This PR fixes the end to end tests by replacing the deprecated Protractor library with Cypress.

This PR also replaces karma and jasmine with Jest [Jest + Angular guide].

Screen Shot 2022-06-07 at 1 37 52 PM

Tests

  • The following tests are written for this issue:
cypress/e2e/spec.cy.ts

[x] The following is the result of the "mvn test" command on the appropriate module:

N/A, helix-front only change. No Java changes.

Changes that Break Backward Compatibility

[x] No backwards incompatible changes

Documentation

[x] New commands documented in ./helix-front/README.md

Commits

[x] Commits match Apache style

Code Quality

[x] My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)

N/A, helix-front only change. No Java changes. See #2143 for an effort to add automatic code formatting for helix-front.

@micahstubbs
Copy link
Contributor Author

@somecodemonkey this PR is ready for your review. Thanks for sharing your frontend expertise!

},
"test": {
"builder": "@angular-devkit/build-angular:karma",
"builder": "@angular-builders/jest:run",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed this guide to migrate, adjusting the paths to helix-front's existing paths along the way.

.subscribe(
result => {
_.forEach(result, (resource) => {
lodashForEach(result, (resource) => {
Copy link
Contributor Author

@micahstubbs micahstubbs Jun 7, 2022

Choose a reason for hiding this comment

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

A workaround for this TypeScript related error: https://stackoverflow.com/q/49256040/1732222

A secondary benefit of this approach is new contributors will be less likely to confuse lodash's forEach with the native TypeScript Array.forEach().

"target": "es2015",
"skipLibCheck": true,
"esModuleInterop": true,
"allowSyntheticDefaultImports": 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.

These two options are the other part of the solution to https://stackoverflow.com/q/49256040/1732222

Copy link
Contributor Author

@micahstubbs micahstubbs left a comment

Choose a reason for hiding this comment

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

Self-review complete. I left some comments for other reviewers. LGTM.

@micahstubbs micahstubbs force-pushed the 2075/e2e-tests-chrome-issue branch from 3b7a936 to cea3e24 Compare June 9, 2022 20:57
@micahstubbs
Copy link
Contributor Author

micahstubbs commented Jun 14, 2022

@somecodemonkey here is the test failure. It seems like installing this frontend dependency vis-network fails in the Ubuntu CI machine. This failure also appears on master:

[INFO] --- frontend-maven-plugin:1.12.1:yarn (yarn install) @ helix-front ---
[INFO] Running 'yarn install' in /home/runner/work/helix/helix/helix-front
[INFO] yarn install v1.22.18
[INFO] [1/5] Validating package.json...
[INFO] [2/5] Resolving packages...
[INFO] warning request > har-validator@5.1.5: this library is no longer supported
[INFO] [3/5] Fetching packages...
[INFO] [4/5] Linking dependencies...
[INFO] warning " > ngx-vis@3.1.0" has incorrect peer dependency "keycharm@^0.2.0".
[INFO] warning "@angular-builders/jest > jest-preset-angular > ts-jest@27.1.5" has incorrect peer dependency "jest@^27.0.0".
[INFO] warning " > @angular/compiler-cli@13.3.9" has incorrect peer dependency "@angular/compiler@13.3.9".
[INFO] warning nohoist config is ignored in "cypress" because it is not a private package. If you think nohoist should be allowed in public packages, please submit an issue for your use case.
[INFO] [5/5] Building fresh packages...
[INFO] info This package requires node-gyp, which is not currently installed. Yarn will attempt to automatically install it. If this fails, you can run "yarn global add node-gyp" to manually install it.
[INFO] error An unexpected error occurred: "/home/runner/work/helix/helix/helix-front/node_modules/vis-network: Cannot read property 'config' of undefined".
[INFO] error An unexpected error occurred: "/home/runner/work/helix/helix/helix-front/node_modules/vis-network: Cannot read property 'config' of undefined".info If you think this is a bug, please open a bug report with the information provided in "/home/runner/.config/yarn/global/yarn-error.log".****

[INFO] error An unexpected error occurred: "/home/runner/work/helix/helix/helix-front/node_modules/vis-network: Cannot read property 'config' of undefined". [INFO] error An unexpected error occurred: "/home/runner/work/helix/helix/helix-front/node_modules/vis-network: Cannot read property 'config' of undefined".info If you think this is a bug, please open a bug report with the information provided in "/home/runner/.config/yarn/global/yarn-error.log".****

https://github.com/apache/helix/runs/6820814450?check_suite_focus=true

logged as #2148

@somecodemonkey
Copy link

somecodemonkey commented Jun 14, 2022

LGTM, please look into the failing build test as well

@micahstubbs
Copy link
Contributor Author

This PR is ready to be merged, approved by @somecodemonkey
Final commit message:

fix e2e tests for helix-front (#2075)

Fix the helix-front end to end tests by replacing the deprecated Protractor testing library with Cypress

@junkaixue
Copy link
Contributor

How was the failing test as Darby mentioned?

@micahstubbs
Copy link
Contributor Author

micahstubbs commented Jun 14, 2022

@junkaixue good question. Details here: #2148

It looks like there is an OS specific failure during Javascript dependency installation.

@micahstubbs
Copy link
Contributor Author

Based on conversation with @junkaixue will fix #2148 first.

@micahstubbs
Copy link
Contributor Author

micahstubbs commented Jun 15, 2022

#2148 is now fixed.

@micahstubbs
Copy link
Contributor Author

micahstubbs commented Jun 15, 2022

This PR is ready to be merged, approved by @somecodemonkey

Final commit message:

fix e2e tests for helix-front (#2075)
Fix the helix-front end to end tests by replacing the deprecated Protractor testing library with Cypress

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.

helix-front e2e tests fail: SessionNotCreatedError: session not created: Chrome version must be between 71 and 75

3 participants