Skip to content

Actor object in the step definition is not the same object that was created in the ParameterType definition #1532

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.

Sign up for GitHub

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

Jump to bottom
Closed
aurelien-reeves opened this issue May 5, 2021 · 3 comments · Fixed by #1538
Closed

Actor object in the step definition is not the same object that was created in the ParameterType definition #1532

aurelien-reeves opened this issue May 5, 2021 · 3 comments · Fixed by #1538
Assignees
@aurelien-reeves
Labels
🐛 bug Defect / Bug 🥒 core team Candidate for going onto the Cucumber Open Board: https://github.com/orgs/cucumber/projects/8 ✅ accepted The core team has agreed that it is a good idea to fix this
Milestone
6.1.0

Comments

@aurelien-reeves
Copy link
Contributor

https://gist.github.com/vincent-psarga/524cedeef9165b8ee0c66a7989cf70d7

Also in shouty:
vincent-psarga/shouty.rb@015bf6e

Given Lucy is at 0, 0        # features/step_definitions/shout_steps.rb:10
   Actor in parameter type: #<Actor:0x00007f9f984c5650>
   Actor in step definition: #<Actor:0x00007f9f984c4d40>

It has been introduced here: 1b9ab0f

More context: #760
@aurelien-reeves aurelien-reeves added 🐛 bug Defect / Bug 🥒 core team Candidate for going onto the Cucumber Open Board: https://github.com/orgs/cucumber/projects/8 ✅ accepted The core team has agreed that it is a good idea to fix this labels May 5, 2021
@aurelien-reeves aurelien-reeves self-assigned this May 11, 2021
@aurelien-reeves
Copy link
Contributor Author

@aslakhellesoy @mattwynne @tooky
Since the introduction of the deep clone for step arguments, the following commit delegates the parameter type transformers to the World object 144f341

That way, it seems the deep clone is no more required.

I did some tests, and indeed, simply removing the deep clone seems to fix the issue still preventing undesirable state leaks.

I'll submit a PR soon.

WDYT?

aurelien-reeves added a commit that referenced this issue May 11, 2021
@aurelien-reeves
Remove deep clone of step arguments

Verified

This commit was signed with the committer’s verified signature.
aurelien-reeves Aurélien Reeves
GPG key ID: 93B547AAE317F671
Verified
Learn about vigilant mode
75e80a9 
Fixes #1532

Step arguments are delegated to the World since 144f341.
That prevent state to leak so the deep-clone seems not required anymore.
@aurelien-reeves aurelien-reeves mentioned this issue May 11, 2021
Merged
@aurelien-reeves
Copy link
Contributor Author

I've submitted the PR #1538
But actually I have no idea how to test it properly, or how to make sure state effectively don't unexpectedly leak.

aurelien-reeves added a commit that referenced this issue May 12, 2021
@aurelien-reeves @mattwynne
Remove deep clone of step arguments (#1538)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
3b03852 
* Remove deep clone of step arguments

Fixes #1532

Step arguments are delegated to the World since 144f341.
That prevent state to leak so the deep-clone seems not required anymore.

Co-authored-by: Matt Wynne <matt@cucumber.io>
@mattwynne mattwynne added this to the 6.1 milestone May 12, 2021
@aurelien-reeves
Copy link
Contributor Author

@vincent-psarga cucumber@6.1.0 should fix this :)

@mattwynne mattwynne mentioned this issue Jun 17, 2021
Open
@mattwynne mattwynne moved this to Released in Cucumber Open Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aurelien-reeves aurelien-reeves

Labels
🐛 bug Defect / Bug 🥒 core team Candidate for going onto the Cucumber Open Board: https://github.com/orgs/cucumber/projects/8 ✅ accepted The core team has agreed that it is a good idea to fix this
Projects
Cucumber Open
Archived in project
Milestone
6.1.0
Development

Successfully merging a pull request may close this issue.

Remove deep clone of step arguments cucumber/cucumber-ruby
2 participants
@mattwynne @aurelien-reeves