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

Fix uninitialized constant RSpec::Rails::OpenStruct error #2754

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

javierjulio
Copy link
Contributor

This started to occur on recent test suite runs e.g. #2752, #2753. It may be related to a json gem update (example) that removed the ostruct dependency as I encountered that with turbo_tests which I use to run my rspec test suite.

This started to occur on recent test suite runs. It may be related to a json gem update that removed the ostruct dependency as I encountered this with turbo_tests and a fix for that is in serpapi/turbo_tests#49 with more details.
@pirj pirj merged commit 8cee7cb into rspec:main Apr 8, 2024
17 checks passed
@pirj
Copy link
Member

pirj commented Apr 8, 2024

Thank you!

@javierjulio
Copy link
Contributor Author

@pirj thank you for the quick turnaround! I'll go back to update the other PR and see what errors remain.

@JonRowe
Copy link
Member

JonRowe commented Apr 8, 2024

@pirj this needs to be reverted or the require moved to a sub process... it could cause dependency based issues for us / our users

@javierjulio
Copy link
Contributor Author

javierjulio commented Apr 8, 2024

@JonRowe I can try to help. Why would it need to be reverted if the require 'ostruct' is just in a spec file that only affects the test suite? I can submit a revert if it's really necessary. Do you have an example of what I can follow for require usage to resolve the OpenStruct error in a safe manner?

@pirj
Copy link
Member

pirj commented Apr 8, 2024

We could just use a Class.new to create an interface that that view helper needs.

@javierjulio
Copy link
Contributor Author

That would be ideal over using OpenStruct since it's been a long time now that Ruby has advised to not use it. I'm not familiar with the requirements for this project so likely missing something. Can I help with a PR to do that instead?

@@ -187,7 +188,7 @@ def _default_file_to_render; end # Stub method
Class.new do
include ViewExampleGroup::ExampleMethods
def controller
@controller ||= OpenStruct.new(params: nil)
@controller ||= ::OpenStruct.new(params: nil)
Copy link
Member

Choose a reason for hiding this comment

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

I think this can become just Struct.new(:params).new(nil), and we’re openstruct-free with no side effects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I will submit a PR with that change to start the replacement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pirj I made this change in #2755

javierjulio added a commit to javierjulio/rspec-rails that referenced this pull request Apr 9, 2024
@JonRowe
Copy link
Member

JonRowe commented Apr 9, 2024

the require 'ostruct' is just in a spec file that only affects the test suite?

The reason for this policy is that it means we could inadvertandly then use openstruct and cause breakages.

The replacement in #2755 is much better thank you.

@javierjulio
Copy link
Contributor Author

Yes, I think it's better too to avoid OpenStruct if possible. Thanks!

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.

3 participants