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

Test suite fails when the host RubyGems are configured via operating_system.rb #4446

Closed
voxik opened this issue Mar 10, 2021 · 11 comments
Closed
Labels

Comments

@voxik
Copy link
Contributor

voxik commented Mar 10, 2021

I am observing following test failures on Fedora:

$ rake 'TESTOPTS=- -v "-e /test_manifest_is_up_to_date/"'
fatal: not a git repository (or any of the parent directories): .git
Ignoring json-2.5.1 because its extensions are not built. Try: gem pristine json --version 2.5.1
Ignoring json-2.5.1 because its extensions are not built. Try: gem pristine json --version 2.5.1
Run options: -v "-e /test_manifest_is_up_to_date/" --seed 27794

# Running:

TestGemCommandsBuildCommand#test_execute_missing_file = 0.01 s = .

... snip ...

  7) Failure:
TestGemExtExtConfBuilder#test_class_build [/builddir/build/BUILD/rubygems-3.2.13/test/rubygems/test_gem_ext_ext_conf_builder.rb:38]:
--- expected
+++ actual
@@ -1,2 +1,4 @@
+"Ignoring json-2.5.1 because its extensions are not built. Try: gem pristine json --version 2.5.1
+" +
 "creating Makefile
 "


  8) Failure:
TestGemExtExtConfBuilder#test_class_build_rbconfig_make_prog [/builddir/build/BUILD/rubygems-3.2.13/test/rubygems/test_gem_ext_ext_conf_builder.rb:61]:
--- expected
+++ actual
@@ -1,2 +1,4 @@
+"Ignoring json-2.5.1 because its extensions are not built. Try: gem pristine json --version 2.5.1
+" +
 "creating Makefile
 "


  9) Failure:
TestGemExtExtConfBuilder#test_class_build_env_make [/builddir/build/BUILD/rubygems-3.2.13/test/rubygems/test_gem_ext_ext_conf_builder.rb:87]:
--- expected
+++ actual
@@ -1,2 +1,4 @@
+"Ignoring json-2.5.1 because its extensions are not built. Try: gem pristine json --version 2.5.1
+" +
 "creating Makefile
 "


2244 runs, 7142 assertions, 3 failures, 0 errors, 6 skips
rake aborted!

The issue here is that Fedora is using operating_system.rb to configure the gem locations. Unfortunately, the test suite fails on various places with that configuration (66 failures, 26 errors). Trying to workaround the issue, I'm placing the empty operating_system.rb into the upstream sources:

mkdir -p lib/rubygems/defaults
touch lib/rubygems/defaults/operating_system.rb

This mostly helps, but leaves the error behind. The reason is that Gem::StubSpecification#missing_extensions? cannot find the gem.build_complete:

def missing_extensions?
return false if default_gem?
return false if extensions.empty?
return false if File.exist? gem_build_complete_path
to_spec.missing_extensions?
end

So far, I have not figured out how to fix/workaround this issue other then disable the failing test cases. The right solution IMO would be if the test suite does not rely on RubyGems populating the load path. Other nice thing would be if RubyGems::TestCase would not require Simplecov.

Anecdotally, there should be similar issues reported also for Psych and io-console and possibly other gems. They are not, but I think seems they are going to pop up soon

@voxik voxik added the RubyGems label Mar 10, 2021
@voxik
Copy link
Contributor Author

voxik commented Mar 10, 2021

Just FTR, the best thing would actually be if the gem.build_complete is not required at all, because it says just that the build was done, not that the library is still there and requirable. I was thinking about e.g. replacing the Specification#extensions after build, so it would refer to the resulting .so files, but I am afraid there is no way to reliably list them.

@deivid-rodriguez
Copy link
Member

The issue about operating_system.rb sounds like something you should patch on your side to me.

The remaining issue maybe was fixed by #4384?

Regarding simplecov, I'm not sure any maintainer currently uses it for this repo. I don't, so I would be happy removing it.

@voxik
Copy link
Contributor Author

voxik commented Mar 11, 2021

The issue about operating_system.rb sounds like something you should patch on your side to me.

I have found workaround:

cat >> lib/simplecov.rb << EOF
class SimpleCov
  def self.method_missing(*args)
  end
end
EOF

That is faking SimpleCov availability, therefore the whole RubyGems loading machinery is never triggered, because the original Ruby require succeeds.

The remaining issue maybe was fixed by #4384?

This ^^ is Bundler change while I am dealing with RubyGems.

Regarding simplecov, I'm not sure any maintainer currently uses it for this repo. I don't, so I would be happy removing it.

I don't have cycles to try to remove it. I'd be just happy if the take away is that this is not good practice.

@voxik
Copy link
Contributor Author

voxik commented Mar 11, 2021

The issue about operating_system.rb sounds like something you should patch on your side to me.

I have found workaround:

cat >> lib/simplecov.rb << EOF
class SimpleCov
  def self.method_missing(*args)
  end
end
EOF

No I did not. I spoke too early 😿

@voxik
Copy link
Contributor Author

voxik commented Mar 11, 2021

The issue about operating_system.rb sounds like something you should patch on your side to me.

I have found workaround:

cat >> lib/simplecov.rb << EOF
class SimpleCov
  def self.method_missing(*args)
  end
end
EOF

No I did not. I spoke too early 😿

So this needs also RUBYOPT="-I$(ruby -rrake -rminitest -rrdoc -rwebrick -e 'puts $LOAD_PATH.join ?:')", which puts all the dependencies on $LOAD_PATH, therefore the plain Ruby require succeeds without using the RubyGems functionality. However, while it fixes the 3 test cases, it breaks another one 🤣

@voxik
Copy link
Contributor Author

voxik commented Mar 11, 2021

Ok, here is the final version:

RUBYOPT="-I$(ruby -e 'size = $LOAD_PATH.size; %w(rake minitest rdoc webrick).each {|r| require r}; puts $LOAD_PATH[...-size].join ?:')"

@voxik voxik closed this as completed Mar 11, 2021
@voxik voxik reopened this Mar 11, 2021
@voxik
Copy link
Contributor Author

voxik commented Mar 11, 2021

Ups, closed just accidentally.

@deivid-rodriguez
Copy link
Member

It sounds like a workaround was found and I'm not sure what we should be changing on our side, so I'll close this.

@voxik
Copy link
Contributor Author

voxik commented Apr 30, 2021

This is what I proposed:

The right solution IMO would be if the test suite does not rely on RubyGems populating the load path.

Really, I don't think it is good idea to let the code which is tested fiddle with the load path. That is not robust. That is what the workaround avoids. This circles back to the --disable-gems we had previously.

@deivid-rodriguez
Copy link
Member

Feel free to propose any changes you see fit.

@deivid-rodriguez
Copy link
Member

Oh, sorry, so what you are proposing is #4440? That was rejected, unfortunately. If you have other improvements to suggest though, feel free to create a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants