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

BUGFIX: Dont initialize $currentContentStreamId by default in behat tests #5162

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Jun 23, 2024

Related #5034

With #5040
the step I am in the active content stream of workspace "live" was turned into I am in workspace "live".

That is problematic as the step

 And I am in workspace "live"

now hardcodes the $currentContentStreamId making assertions after changing the workspaces content stream (publish or discard) seem at first broken.

A current workaround is to repeat the step after discarding for example:

When the command DiscardWorkspace is executed with payload:
  | Key           | Value         |
  | workspaceName | "user-editor" |
# FIXME we have to reevaluated the step as we cache the $currentContentStreamId and it will be outdated after the discard
And I am in workspace "user-editor" and dimension space point {}
Then I expect node aggregate identifier "text-node-middle" to lead to no node

... but all test pass after these adjustments so id say we dont need the hack in the first place.

Upgrade instructions

Review instructions

Checklist

  • Code follows the PSR-2 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

@mhsdesign mhsdesign force-pushed the bugfix/dont-initialize-current-content-stream-id-by-default-in-tests branch from 5f94351 to 78e0f89 Compare June 23, 2024 18:07
@mhsdesign mhsdesign force-pushed the bugfix/dont-initialize-current-content-stream-id-by-default-in-tests branch from 78e0f89 to 0f199f3 Compare June 24, 2024 06:53
@mhsdesign mhsdesign requested review from nezaniel and grebaldi June 24, 2024 06:56
Copy link
Contributor

@grebaldi grebaldi left a comment

Choose a reason for hiding this comment

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

👍 by 👀

@pKallert pKallert self-requested a review June 27, 2024 12:44
Copy link
Contributor

@pKallert pKallert left a comment

Choose a reason for hiding this comment

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

Looks good 👍 I did not check all occurences (but most of them) but I guess it also will not break any tests where it is currently used 🙂

@mhsdesign mhsdesign merged commit acbacd7 into neos:9.0 Jun 27, 2024
9 checks passed
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this pull request Jun 28, 2024
@mhsdesign mhsdesign deleted the bugfix/dont-initialize-current-content-stream-id-by-default-in-tests branch June 28, 2024 20:09
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this pull request Jun 29, 2024
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this pull request Jul 1, 2024
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