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: Workspace based tests #5040

Merged
merged 3 commits into from
May 11, 2024
Merged

TASK: Workspace based tests #5040

merged 3 commits into from
May 11, 2024

Conversation

nezaniel
Copy link
Member

@nezaniel nezaniel commented May 11, 2024

Resolves partially #5034

This adjusts the test suite to operate directly on workspace names instead of content stream IDs

Upgrade instructions

Review instructions

Most changes are purely search/replace, with a few exceptions:

https://github.com/neos/neos-development-collection/compare/5034-workspaceBasedTests?expand=1#diff-2e50430c540d541e90b7b2233957a44a9d2fb4e8240d3de077bbc93192d3cc65R306

Here we check the timestamps in the new content stream instead of the closed one as before, thus the change in payload, as discussed with @bwaidelich in Slack

https://github.com/neos/neos-development-collection/compare/5034-workspaceBasedTests?expand=1#diff-a3b3697e967ca37986b11850c79037b9594ba0742f7cf2da5a140f31f85701a3L81

Has been refactored to use the given content stream id and the matching workspace instead of the unrelated current workspace name.

Also, we cannot remove content stream checks as we have test cases for content stream forking without workspaces being involved. Also, structure adjustments don't use workspaces, so we have to again check content streams

Checklist

  • Code follows the PSR-12 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

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.

Looks perfect ❤️ , went through all the php files by reading and the string replaces looking good also ;)

... btw i cannot seem to find out where the second link in your pr points to?

@nezaniel
Copy link
Member Author

I guess it's just too much for Github...
it should lead to ProjectedNodeTrait::iExpectANodeIdentifiedByXToExistInTheContentGraph

@mhsdesign
Copy link
Member

Did a reverse string replace and reviewed that diff fully just for fun. Can confirm that only things have been touched that should have and the time stamp thing. Will merge this now as this blocks progress ;)

@mhsdesign mhsdesign merged commit 8916eab into 9.0 May 11, 2024
13 checks passed
@mhsdesign mhsdesign deleted the 5034-workspaceBasedTests branch May 11, 2024 12:01
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.

3 participants