search

serpapi / turbo_tests

Run RSpec tests on multiple cores. Like parallel_tests but with incremental summarized output. Originally extracted from the Discourse and Rubygems source code.
https://rubygems.org/gems/turbo_tests
MIT License
173 stars 25 forks source link

Explicitely load ostruct so it's compatible with latest JSON gem #49

Closed lcmen closed 4 months ago

lcmen commented 4 months ago

The latest json gem version (2.7.2 for the time being) no longer requires OpenStruct (PR: https://github.com/flori/json/pull/565/files) making this gem incompatible with it.

This PR modifies the codebase to explicitly require 'ostruct' so JSON gem is aware of it but most importantly it makes OpenStruct available in lib/utils/hash_extension.

inkstak commented 4 months ago

I've also had to fixed it in #41 (but an explicit PR might be better).

lcmen commented 4 months ago

@javierjulio @deivid-rodriguez who can I ping to merge it and release a new version?

bmulholland commented 4 months ago

Closes https://github.com/serpapi/turbo_tests/issues/52, which I created because such an issue would have saved me 15 minutes (I looked only in Issues, not in PRs).

lcmen commented 4 months ago

Thank you @ilyazub !

ilyazub commented 4 months ago

Released in v2.2.1: https://rubygems.org/gems/turbo_tests/versions/2.2.1.

hsbt commented 4 months ago

Thanks all ❤️ (I'm maintainer of json gem)

ilyazub commented 4 months ago

This PR introduced a problem in Ruby-core CI: https://github.com/ruby/ruby/pull/10496.

javierjulio commented 4 months ago

@ilyazub I encountered the same JSON gem related error in the rspec-rails repo when submitting a PR. I resolved that issue initially by requiring OpenStruct but since it was only one instance, I replaced it later with a Struct instead due to maintainer's suggestion. I wonder if it would help to do that same replacement here too? In general, it's long been advised to not use OpenStruct anyway.

javierjulio commented 4 months ago

@ilyazub I have a patch in https://github.com/serpapi/turbo_tests/pull/53 ready for review that should replace the OpenStruct with a Struct instead so the ostruct dependency can be removed safely.

ilyazub commented 4 months ago

@javierjulio Makes sense. Thank you for debugging it!

I encountered the same JSON gem related error in the rspec-rails repo when submitting a PR.

I understood the consequences (rspec-rails/ruby-core had issues), but not the actual root cause. What error it was in the rspec-rails repo?

javierjulio commented 4 months ago

@ilyazub you're welcome! It was the same error as reported here, essentially a "NameError: uninitialized constant OpenStruct" error message. This is a failed build on rspec-rails from my first PR where that error occurred. I'm not sure I understand the root cause but it's tied to the json gem somehow. The understanding I and others came to elsewhere was it would be best to remove the OpenStruct usage rather than continue to rely on it.

ilyazub commented 4 months ago

Thank you @javierjulio! https://github.com/rspec/rspec-rails/pull/2755 makes total sense.

lcmen commented 4 months ago

Thank you guys for addressing from bigger picture perspective 🙇 . I wasn't aware of the problem in other repos.