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

Three tests fail in Fedora packaging environment #4429

Closed
voxik opened this issue Mar 5, 2021 · 12 comments
Closed

Three tests fail in Fedora packaging environment #4429

voxik opened this issue Mar 5, 2021 · 12 comments
Labels

Comments

@voxik
Copy link
Contributor

voxik commented Mar 5, 2021

Trying to update rubygems package on Fedora, I observe following error:

  1) Failure:
TestKernel#test_gem_failing_inside_require_doesnt_cause_double_exceptions [/builddir/build/BUILD/rubygems-3.2.13/test/rubygems/test_kernel.rb:107]:
Expected: 1
  Actual: 0

Digging deeper, this is the actual issue:

$ ruby -e 'Dir.glob "./test/**/test_kernel.rb", &method(:require)' -- -n /test_gem_failing_inside_require_doesnt_cause_double_exceptions/
Ignoring json-2.5.1 because its extensions are not built. Try: gem pristine json --version 2.5.1
Run options: -n /test_gem_failing_inside_require_doesnt_cause_double_exceptions/ --seed 11053

# Running:


From: /builddir/build/BUILD/rubygems-3.2.13/test/rubygems/test_kernel.rb @ line 104 :

     99:       { "GEM_HOME" => Gem.paths.home },
    100:       *ruby_with_rubygems_in_load_path,
    101:       "-r",
    102:       "./activate.rb"
    103:     )
 => 104: binding.irb
    105:     load_errors = output.split("\n").select {|line| line.include?("Could not find") }
    106: 
    107:     assert_equal 1, load_errors.size
    108:   end
    109: 

irb(#<TestKernel:0x000055873bfe3180>):001:0> p output
"/builddir/build/BUILD/rubygems-3.2.13/tmp/test_rubygems_20210305-31-sxdh6q/activate.rb:1:in `<top (required)>': undefined method `gem' for main:Object (NoMethodError)\n\tfrom -:in `require'\n"
=> "/builddir/build/BUILD/rubygems-3.2.13/tmp/test_rubygems_20210305-31-sxdh6q/activate.rb:1:in `<top (required)>': undefined method `gem' for main:Object (NoMethodError)\n\tfrom -:in `require'\n"
irb(#<TestKernel:0x000055873bfe3180>):002:0> 
F

Finished in 2499.142949s, 0.0004 runs/s, 0.0004 assertions/s.

  1) Failure:
TestKernel#test_gem_failing_inside_require_doesnt_cause_double_exceptions [/builddir/build/BUILD/rubygems-3.2.13/test/rubygems/test_kernel.rb:107]:
Expected: 1
  Actual: 0

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips

Since there is used export RUBYOPT="--disable-gems -Ilib:test:bundler/lib" prior the test execution this is not completely surprising and adding -rrubygems into the Open3 call fixes the situation. Now I wonder, is there a chance to have this fixed upstream?

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

voxik commented Mar 5, 2021

Just FTR, the update is from 3.1.4 where the test suite was passing just fine with the --disable-gems and the option is used since 2.6.10.

Now trying to drop the option, different set of test cases fail, so the test suite is not passing with or without that option :/

@deivid-rodriguez
Copy link
Member

Sure, feel free to propose a fix. Just make sure this test still fails if the patch that introduced it is reverted.

@deivid-rodriguez deivid-rodriguez changed the title TestKernel#test_gem_failing_inside_require_doesnt_cause_double_exceptions One test fails in Fedora packaging environment Mar 5, 2021
@deivid-rodriguez
Copy link
Member

The fix you proposed of adding -rrubygems makes sense to me.

@voxik
Copy link
Contributor Author

voxik commented Mar 8, 2021

Ah, now I just noticed that there are failing also other two test cases for similar reason. However, there was explicitly removed the -rrubygems in 583316b

@deivid-rodriguez deivid-rodriguez changed the title One test fails in Fedora packaging environment Three tests fail in Fedora packaging environment Mar 8, 2021
@deivid-rodriguez
Copy link
Member

I didn't know that you run tests like this at the time, so I thought it was not needed. Feel free to add it back.

@simi
Copy link
Member

simi commented Mar 8, 2021

@deivid-rodriguez do we (want to) support tests running with --disable-gems at all?

@deivid-rodriguez
Copy link
Member

I don't have a strong opinion, but if rubygems packagers want to keep that working in order to be able to run all tests without having to modify their copy of rubygems, I'm happy to accept contributions in that regard.

@voxik
Copy link
Contributor Author

voxik commented Mar 8, 2021

@deivid-rodriguez do we (want to) support tests running with --disable-gems at all?

Actually this is question also I have on my mind. I think that ruining the test suite with --disable-gems was historically the thing, because simply older Ruby had not included RubyGems. Also, I think that running the test suite with --disable-gems might limit the possible interference between "system" RubyGems and the upstream version.

OTOH, this configuration is not tested.

However, I have checked Fedora and it seems that it is possible to execute the test suite via rake after all and since there are test cases depending on Rake, it needs to be available. Therefore I think the right thing is to answer the --disable-gems question, no matter what Fedora does.

@deivid-rodriguez
Copy link
Member

The answer(s) to that question would be

Do we support tests running with --disable-gems at all?

No, since we don't test it and take no special care of not breaking it.

Do we want to support tests running with --disable-gems at all?

I don't have a strong opinion. If someone fixes it and adds some bare tests to keep it working, I wouldn't mind supporting it.

@voxik
Copy link
Contributor Author

voxik commented Mar 10, 2021

Ah, now I have discovered that using --disable-gems does not come from my mind:

b3cb4fb

@deivid-rodriguez
Copy link
Member

That commit doesn't make much sense to me now 😅.

@deivid-rodriguez
Copy link
Member

Closing since we won't be fixing this as per #4440 (comment).

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

3 participants