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

Add cypress to prefix path router #3719

Merged
merged 18 commits into from
Jul 12, 2023

Conversation

sneridagh
Copy link
Member

No description provided.

@netlify
Copy link

netlify bot commented Oct 4, 2022

Deploy Preview for volto canceled.

Name Link
🔨 Latest commit 4129331
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/64ac51aecc414d0008388acf

@sneridagh sneridagh changed the title Add cypress to prefix path router2 Add cypress to prefix path router Oct 4, 2022
@cypress
Copy link

cypress bot commented Oct 4, 2022

6 failed tests on run #7227 ↗︎

6 520 20 0 Flakiness 0

Details:

Remove accesslog
Project: Volto Commit: 412933134c
Status: Failed Duration: 09:25 💡
Started: Nov 25, 2023 4:54 PM Ended: Nov 25, 2023 5:04 PM
Failed  core/blocks/blocks-chooser.js • 1 failed test • Core Blocks 18.x

View Output Video

Test Artifacts
Blocks Tests > Add image block Screenshots Video
Failed  core/blocks/blocks-image.js • 1 failed test • Core Blocks 18.x

View Output Video

Test Artifacts
Blocks Tests > Add image block Screenshots Video
Failed  minimal/blocks-image.js • 1 failed test • Prefix

View Output Video

Test Artifacts
Blocks Tests > Add image block Screenshots Video
Failed  core/blocks/blocks-chooser.js • 1 failed test • Core Blocks 16.x

View Output Video

Test Artifacts
Blocks Tests > Add image block Screenshots Video
Failed  core/blocks/blocks-image.js • 1 failed test • Core Blocks 16.x

View Output Video

Test Artifacts
Blocks Tests > Add image block Screenshots Video

The first 5 failed specs are shown, see all 6 specs in Cypress Cloud.

Review all test suite changes for PR #3719 ↗︎

@sneridagh
Copy link
Member Author

Ok, it does work :) Tests are failing because URL checks that do not expect prefixed paths.

Rather than duplicate tests, I'd suggest we create a Cypress command cy.checkURL that takes if the tests should be prefixed or not, then adapt.

@sneridagh sneridagh mentioned this pull request Oct 4, 2022
@tiberiuichim tiberiuichim added this to the 17.x.x milestone Nov 26, 2022
@nileshgulia1 nileshgulia1 marked this pull request as ready for review December 15, 2022 14:29
@nileshgulia1 nileshgulia1 mentioned this pull request Jan 21, 2023
9 tasks
@cypress
Copy link

cypress bot commented May 17, 2023

Passing run #6176 ↗︎

0 14 0 0 Flakiness 0

Details:

Merge d79b936 into cc4266e...
Project: Volto Commit: 1b431f4604 ℹ️
Status: Passed Duration: 03:05 💡
Started: Jul 7, 2023 4:16 PM Ended: Jul 7, 2023 4:19 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@sneridagh sneridagh modified the milestones: 17.x.x, Future Jun 6, 2023
@wesleybl wesleybl requested a review from stevepiercy July 4, 2023 20:48
@wesleybl
Copy link
Member

wesleybl commented Jul 4, 2023

@stevepiercy can you review it here please? What to do with the error:

https://github.com/plone/volto/actions/runs/5457862287/jobs/9932330404?pr=3719#step:7:156

?

The link in question is:

created and designed by [Albert Casado](https://twitter.com/albertcasado).

@stevepiercy
Copy link
Collaborator

For the error, someone should approve my open PR: #4941

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Some English and MyST syntax and grammar fixes, and a style guide note about one line per sentence in docs. Thank you!

docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
@wesleybl
Copy link
Member

wesleybl commented Jul 5, 2023

@stevepiercy I made the changes. Can you please have a look again?

@wesleybl wesleybl requested a review from stevepiercy July 5, 2023 16:29
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

A couple of corrections. It looks like a couple of bad copy-pastas created spaghetti docs. 🍝

docs/source/deploying/prefixed-root.md Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
docs/source/deploying/prefixed-root.md Outdated Show resolved Hide resolved
@wesleybl
Copy link
Member

wesleybl commented Jul 5, 2023

@stevepiercy Done.

@wesleybl wesleybl requested a review from stevepiercy July 5, 2023 17:50
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Docs are good. Does this PR need code review by a maintainer?

@stevepiercy
Copy link
Collaborator

@wesleybl would you please merge master into this PR to get the docs to pass, now that the PR with a fix has been merged? Thank you!

@wesleybl
Copy link
Member

wesleybl commented Jul 5, 2023

@wesleybl would you please merge master into this PR to get the docs to pass, now that the PR with a fix has been merged?

@stevepiercy done.

@wesleybl wesleybl force-pushed the prefix_path_router branch from 781f428 to cc4266e Compare July 7, 2023 14:49
@wesleybl wesleybl force-pushed the addCypressTo-prefix_path_router2 branch 2 times, most recently from 548e6a0 to bd05860 Compare July 10, 2023 15:40
Copy link
Member

@wesleybl wesleybl left a comment

Choose a reason for hiding this comment

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

LGTM

@wesleybl
Copy link
Member

@sneridagh @nileshgulia1 the tests are working now. Can we merge?

@wesleybl wesleybl requested a review from nileshgulia1 July 10, 2023 16:12
@nileshgulia1
Copy link
Member

Thanks @wesleybl for taking care!

@wesleybl wesleybl force-pushed the prefix_path_router branch from cc4266e to e5e9ed7 Compare July 10, 2023 18:25
@wesleybl wesleybl force-pushed the addCypressTo-prefix_path_router2 branch from b9dfaa0 to 4129331 Compare July 10, 2023 18:45
@wesleybl wesleybl merged commit 7d2e756 into prefix_path_router Jul 12, 2023
@wesleybl wesleybl deleted the addCypressTo-prefix_path_router2 branch July 12, 2023 12:19
@wesleybl
Copy link
Member

This PR was merged into #3592

wesleybl added a commit that referenced this pull request Jul 12, 2023
Co-authored-by: nileshgulia1 <nileshgulia@gmail.com>
Co-authored-by: wesleybl <wesleybl@gmail.com>
@nileshgulia1 nileshgulia1 restored the addCypressTo-prefix_path_router2 branch November 25, 2023 16:52
@nileshgulia1 nileshgulia1 deleted the addCypressTo-prefix_path_router2 branch November 25, 2023 16:57
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.

5 participants