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

TASK: Make E2E tests work for Neos 9.0 #3569

Merged
merged 31 commits into from
Sep 23, 2023
Merged

TASK: Make E2E tests work for Neos 9.0 #3569

merged 31 commits into from
Sep 23, 2023

Conversation

grebaldi
Copy link
Contributor

@grebaldi grebaldi commented Jul 7, 2023

refs: #3546, #3406
includes: #3571
includes: #3618

This is the third and final in a series of PRs leading up to Neos 9.0. I want to make sure that our E2E tests can be executed locally on every maintained branch.

How to test this

You should of course be able to run the E2E tests locally via

make test-e2e-docker browser=chromium # or browser=chrome

The E2E tests use cweagans/composer-patches to include patches from the development collection. Documentation can be found here: https://github.com/cweagans/composer-patches/blob/1.x/README.md
We discussed to keep this dependency to make developing across repos easier ;)

Remaining TODOs

grebaldi added 2 commits July 7, 2023 12:28
This includes the replacement of the 1Dimension site export with an equivalent
ESCR event export, as well as the adjustment of the e2e-docker.sh script for
bootstrapping the test environment.
grebaldi added 2 commits July 8, 2023 12:34
Using the `PendingChangesProjection` from Neos.Neos, which after
neos/neos-development-collection#4393 will track all
nodes that have been newly created within the workspace that contains the
change set that is being discarded.
mficzel and others added 6 commits July 9, 2023 16:12
….isEditMode(request) || Neos.Backend.isPreviewMode(request)`
This includes the replacement of the 2Dimension site export with an equivalent
ESCR event export.

Also, as a temporary fix,
neos/neos-development-collection#4395 was added as a
patch.
As it turns out, using nodeAddresses rather than context paths, these feedbacks
are no longer necessary.
With this change, the UI will default to the default inPlace edit mode,
regardless of user-side selection.

Actual full-blown support for EditPreviewModes will require deeper changes
over the entire code base.
@grebaldi grebaldi force-pushed the task/e2e-tests-on-9-0 branch 2 times, most recently from 30cf764 to 58cfe98 Compare July 11, 2023 12:00
@grebaldi grebaldi force-pushed the task/e2e-tests-on-9-0 branch from 58cfe98 to e018877 Compare July 11, 2023 12:15
@grebaldi grebaldi force-pushed the task/e2e-tests-on-9-0 branch from e4356fe to 45c2cf6 Compare July 11, 2023 15:32
@grebaldi grebaldi force-pushed the task/e2e-tests-on-9-0 branch from 45c2cf6 to 895ca5c Compare July 11, 2023 15:40
Copy link
Member

@skurfuerst skurfuerst left a comment

Choose a reason for hiding this comment

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

just looked over this - GREAT WORK <3 <3 I'll check that we get the dependent PRs merged soon, so that we can get this one in.

@mhsdesign
Copy link
Member

As there might be a collision as you pointed out, i will take care of reviewing and merging: #3503

Also it seems like neos/neos-development-collection#4067 is blocking this pr from moving forward so we should definitely focus there as well ;) But we discussed in the last escr meeting a plan for the preview modes its going forward - slowly ^^

@mhsdesign mhsdesign self-requested a review August 8, 2023 19:57
Comment on lines +479 to +487
const nodeContextPathElement = d.querySelector('.node-context-path');
if (!nodeContextPathElement) {
throw new Error('.node-context-path is not found in the result');
}

const nodeContextPath = nodeContextPathElement.innerHTML.trim();
if (!nodeContextPath) {
throw new Error('.node-context-path is empty');
}
Copy link
Member

Choose a reason for hiding this comment

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

why did this work previously ??? 😂

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 suppose .node-context-path was never empty before, but occurred to be empty during the tests, because of all the backend changes.

@@ -142,7 +142,7 @@ export function * watchCurrentDocument({globalRegistry, configuration}) {
}

const state = yield select();
const childrenAreFullyLoaded = $get(['cr', 'nodes', 'byContextPath', contextPath, 'children'], state)
const childrenAreFullyLoaded = ($get(['cr', 'nodes', 'byContextPath', contextPath, 'children'], state) || [])
Copy link
Member

Choose a reason for hiding this comment

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

this fix could be backported as well no?

(would also take care of it just vote: 👍 👎 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My plan was to leave the plow-js calls as they are for now and take care of replacing them in #3306, right after the E2E tests have been merged.

@mhsdesign
Copy link
Member

I did revert your hotfix for edit preview modes ce395ca

and merged this final implementation: #3618 into your branch

lets see what the tests say :D

@mhsdesign
Copy link
Member

mhsdesign commented Sep 21, 2023

I added a hack so the tests should now work in ci :D - will take care of a proper fix.

locally make test-e2e-docker doesnt seem to work as i think i have problems with http://onedimension.localhost:8081/neos and http://twodimensions.localhost:8081/neos

-> it seems one needs to modify the /etc/hosts ?

          echo 127.0.0.1 onedimension.localhost | sudo tee -a /etc/hosts
          echo 127.0.0.1 twodimensions.localhost | sudo tee -a /etc/hosts

so we need a tutorial for this for mac as well ;)

Copy link
Member

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

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

I just ran the tests make test-e2e-docker locally on my mac and they work flawlessly!

As soon as the ci is green we can merge this :D

@mhsdesign mhsdesign force-pushed the task/e2e-tests-on-9-0 branch 3 times, most recently from 3e0e179 to 50f1a4f Compare September 23, 2023 07:03
@mhsdesign mhsdesign force-pushed the task/e2e-tests-on-9-0 branch from 50f1a4f to dd99278 Compare September 23, 2023 07:04
@mhsdesign mhsdesign merged commit d632b6a into 9.0 Sep 23, 2023
4 checks passed
@mhsdesign mhsdesign deleted the task/e2e-tests-on-9-0 branch September 23, 2023 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants