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 26 forks source link

Windows support #50

Closed deivid-rodriguez closed 6 months ago

deivid-rodriguez commented 7 months ago

We've been running turbo_tests in RubyGems with a monkeypatch to make Windows support work. A unified solution would be better but I guess getting things at least working should take priority so maybe there's interest in the raw patch.

Windows tests don't really catch any issues (they pass without this patch), although turbo_tests does not really work on Windows. In any case, as a minimal improvement over what we have, I added Windows to CI.

Finally, I dropped support for Ruby 2.7 since it reached its End of Life and also started testing with Ruby 3.3.

Happy to extract any of these changes to separate PRs!

deivid-rodriguez commented 6 months ago

I see, let me remove the commit dropping support for Ruby 2.7 then!

deivid-rodriguez commented 6 months ago

Thanks for that! I will give this patch a try and confirm it works for us!

deivid-rodriguez commented 6 months ago

I tried this branch through https://github.com/rubygems/rubygems/commit/a4e301244d3a3b2bfc7771edceb50d453e20cd1b and Windows CI is cool with it.

I also tried it locally on Linux and found no issues either, so this seems good!

Unfortunately we're having an unrelated issue, and we had to pin to 2.2.0 for now, but this seems good to me regardless!

ilyazub commented 6 months ago

Sounds good, thank you @deivid-rodriguez.

I'll merge this PR and will also investigate https://github.com/ruby/ruby/pull/10496.

javierjulio commented 6 months ago

@deivid-rodriguez @ilyazub I created #53 to resolve the OpenStruct issue (from #49) by replacing it with a Struct instead. I believe with that change, the json gem dependency could be removed since it was added in #49 along with OpenStruct.

deivid-rodriguez commented 6 months ago

Thank you! Hopefully we'll be able to upgrade and remove our monkeypatches soon!

ilyazub commented 6 months ago

I reverted #49 in https://github.com/serpapi/turbo_tests/pull/54 and am preparing the PR to fix the Ruby-core tests failure in commands/pristine_spec.rb.

deivid-rodriguez commented 6 months ago

Yeah, that should workaround the issue for us, thank you!

In any case, I think the ruby-core issue reveals some problem in bundler or ruby-core test setup, and I believe it's better to explicitly list dependencies that these days are maintained as separate gems.

This change is good for us since it unblock us from upgrading but if I manage to figure out the problem, I'll make sure to get back here and try to add back the explicit json dependency.

ilyazub commented 6 months ago

@deivid-rodriguez bundle pristine failed in the Ruby-core tests (ref: https://github.com/ruby/ruby/pull/10496#issuecomment-2059313556)

Cannot pristine baz (1.0.0). Gem is sourced from local path.
The installation path is insecure. Bundler cannot continue.
/omitted/path/ruby/tmp/1/gems/system/extensions/x86_64-linux/3.4.0+0-static is world-writable (without sticky bit).
Bundler cannot safely replace gems in world-writeable directories due to potential vulnerabilities.
Please change the permissions of this directory or choose a different install path.

What does the The installation path is insecure error means?

deivid-rodriguez commented 6 months ago

We try to explain it below. Basically means that the previous installation path has insecure world-writable permissions, and since Bundler needs to delete it for bundle pristine, if a malicious user, for example, symlinked a file in that folder to some other important file, Bundler could end up deleting it.

That all said, I don't know how or why the error is being triggered here :open_mouth:.