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 TruffleRuby in CI #2797

Merged
merged 8 commits into from May 12, 2020
Merged

Test TruffleRuby in CI #2797

merged 8 commits into from May 12, 2020

Conversation

eregon
Copy link
Contributor

@eregon eregon commented Jun 19, 2019

Description:

We'd like to test TruffleRuby in RubyGems' CI, but it's not working yet.
This is related to #2700, but with additional fixes to run tests.

cc @hsbt

I will abide by the code of conduct.

@eregon
Copy link
Contributor Author

eregon commented Jun 19, 2019

Is there an easy way to disable AppVeyor from from building this branch?
It's not useful while I'm debugging, and probably makes other builds wait.

@deivid-rodriguez
Copy link
Member

@eregon Not sure but maybe adding test: off to the config will do the trick?

@eregon eregon force-pushed the test-truffleruby branch 2 times, most recently from 573d4c1 to 531d882 Compare July 2, 2019 21:06
@eregon
Copy link
Contributor Author

eregon commented Jul 2, 2019

I rebased this and it should now use the latest 19.1.0 TruffleRuby release, but TravisCI is down, oh well.

@duckinator
Copy link
Member

@eregon it looks like your change to TruffleRuby last month addresses a significant number of the errors in the last Travis CI run. It also appears that 19.1.1 was released after that commit.

So — assuming your changes are in TruffleRuby 19.1.1 — I think if you rebase this, it may have fewer failures.

@eregon
Copy link
Contributor Author

eregon commented Aug 3, 2019

@duckinator 19.1.1 only contains security fixes and since there was none for TruffleRuby, it's exactly the same as the 19.1.0 TruffleRuby standalone. So, we have to wait the 19.2.0 release to get that fix. I don't think that fix alone will be enough.

Please let me work alone on this for now, since I know most about TruffleRuby and I think right now there is little the RubyGems contributors can do to help.
@hsbt Until this is integrated, feel free to ignore TruffleRuby for RubyGems releases.

@eregon
Copy link
Contributor Author

eregon commented Aug 3, 2019

What we really need to make better progress on this is nightly builds (oracle/truffleruby#1483). I'm trying to get those, but it takes a while to put all the pieces together.

It's rather frustrating that RubyGems tests mostly just work when we run a good part of them in TruffleRuby's CI, but for some reason it's a lot harder to make them run in TravisCI.

@duckinator
Copy link
Member

@eregon understood. 👍

@eregon
Copy link
Contributor Author

eregon commented Aug 4, 2019

Making some progress, the tests were somehow expecting #{RUBY_ENGINE.upcase}_VERSION (i.e., TRUFFLERUBY_VERSION) to be defined and hiding the error.
#2864 and rubygems/bundler#7275 solve that, please review those.

ghost pushed a commit that referenced this pull request Aug 5, 2019
2864: Use the standard RUBY_ENGINE_VERSION instead of JRUBY_VERSION r=bronzdoc a=eregon

* RUBY_ENGINE and RUBY_ENGINE_VERSION are defined on every modern Ruby.
* There is no such constant as TRUFFLERUBY_VERSION or RBX_VERSION.

This blocks #2797.
Also see rubygems/bundler#7275

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


Co-authored-by: Benoit Daloze <eregontp@gmail.com>
ghost pushed a commit to rubygems/bundler that referenced this pull request Aug 5, 2019
7275: Use the standard RUBY_ENGINE_VERSION instead of JRUBY_VERSION r=deivid-rodriguez a=eregon

* RUBY_ENGINE and RUBY_ENGINE_VERSION are defined on every modern Ruby.
* There is no such constant as TRUFFLERUBY_VERSION or RBX_VERSION.

This blocks rubygems/rubygems#2797.
Also see rubygems/rubygems#2864

Co-authored-by: Benoit Daloze <eregontp@gmail.com>
hsbt pushed a commit that referenced this pull request Aug 16, 2019
2864: Use the standard RUBY_ENGINE_VERSION instead of JRUBY_VERSION r=bronzdoc a=eregon

* RUBY_ENGINE and RUBY_ENGINE_VERSION are defined on every modern Ruby.
* There is no such constant as TRUFFLERUBY_VERSION or RBX_VERSION.

This blocks #2797.
Also see rubygems/bundler#7275

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


Co-authored-by: Benoit Daloze <eregontp@gmail.com>
hsbt pushed a commit that referenced this pull request Aug 16, 2019
2864: Use the standard RUBY_ENGINE_VERSION instead of JRUBY_VERSION r=bronzdoc a=eregon

* RUBY_ENGINE and RUBY_ENGINE_VERSION are defined on every modern Ruby.
* There is no such constant as TRUFFLERUBY_VERSION or RBX_VERSION.

This blocks #2797.
Also see rubygems/bundler#7275

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).


Co-authored-by: Benoit Daloze <eregontp@gmail.com>
@hsbt
Copy link
Member

hsbt commented Dec 28, 2019

@eregon Can you update with the current master branch? We moved to GitHub Actions instead of Travis CI. See https://github.com/rubygems/rubygems/blob/master/.github/workflows/ubuntu-rvm.yml#L16

@eregon eregon force-pushed the test-truffleruby branch 2 times, most recently from dbc5bae to a2bce0a Compare March 7, 2020 14:51
@eregon eregon changed the title [WIP] Test TruffleRuby in TravisCI [WIP] Test TruffleRuby in CI Mar 7, 2020
@eregon
Copy link
Contributor Author

eregon commented Mar 7, 2020

Making progress here, by running it locally and fixing the missing Errno::ENOTSUP I get:

  1) Failure:
TestGemPackageTarReaderEntry#test_full_name_null [/home/eregon/code/rubygems/test/rubygems/test_gem_package_tar_reader_entry.rb:82]:
Gem::Package::TarInvalidError expected but nothing was raised.

  2) Failure:
TestGemRequire#test_activate_via_require_respects_loaded_files [/home/eregon/code/rubygems/test/rubygems/test_require.rb:242]:
benchmark should have already been loaded

  3) Failure:
TestGemRequire#test_realworld_default_gem [/home/eregon/code/rubygems/test/rubygems/test_require.rb:395]:
Expected "" to not be empty.

  4) Failure:
TestGemRequire#test_no_kernel_require_in_warn_with_uplevel [/home/eregon/code/rubygems/test/rubygems/test_require.rb:528]:
Expected /main\.rb:1: warning: uplevel\ntest\n$/ to match "main.rb:1:in `require': cannot load such file -- sub (LoadError)\n" +
"\tfrom main.rb:1:in `<main>'\n".

  5) Failure:
TestGemRequire#test_no_kernel_require_in_Kernel_warn_with_uplevel [/home/eregon/code/rubygems/test/rubygems/test_require.rb:528]:
Expected /main\.rb:1: warning: uplevel\ntest\n$/ to match "main.rb:1:in `require': cannot load such file -- sub (LoadError)\n" +
"\tfrom main.rb:1:in `<main>'\n".

  6) Failure:
TestStubSpecification#test_contains_requirable_file_eh_extension [/home/eregon/code/rubygems/test/rubygems/test_gem_stub_specification.rb:79]:
--- expected
+++ actual
@@ -1,2 +1 @@
-"Ignoring stub_e-2 because its extensions are not built. Try: gem pristine stub_e --version 2
-"
+""

  7) Skipped:
TestGemExtBuilder#test_build_extensions_extconf_bad [/home/eregon/code/rubygems/test/rubygems/test_gem_ext_builder.rb:244]:
Gem.ruby is not the name of the binary being run in the end

  8) Failure:
TestGem#test_self_try_activate_missing_extensions [/home/eregon/code/rubygems/test/rubygems/test_gem.rb:1305]:
--- expected
+++ actual
@@ -1,2 +1 @@
-"Ignoring ext-1 because its extensions are not built. Try: gem pristine ext --version 1
-"
+""

  9) Failure:
TestGemInstallUpdateOptions#test_add_install_update_options [/home/eregon/code/rubygems/test/rubygems/test_gem_install_update_options.rb:36]:
Expected false to be truthy.

 10) Failure:
TestGemPackage#test_verify_corrupt [/home/eregon/code/rubygems/test/rubygems/test_gem_package.rb:906]:
--- expected
+++ actual
@@ -1 +1 @@
-"tar is corrupt, name contains null byte in /tmp/corrupt20200307-16910-1xnrtgu"
+"package metadata is missing in /tmp/corrupt20200307-16910-1xnrtgu"

 11) Failure:
TestGemSpecification#test_contains_requirable_file_eh_extension [/home/eregon/code/rubygems/test/rubygems/test_gem_specification.rb:1695]:
--- expected
+++ actual
@@ -1,2 +1 @@
-"Ignoring ext-1 because its extensions are not built. Try: gem pristine ext --version 1
-"
+""

 12) Error:
TestGemSpecification#test_self_load_tainted:
SecurityError: Setting $SAFE is no longer supported.
    /home/eregon/code/rubygems/test/rubygems/test_gem_specification.rb:955:in `block in <top (required)>'
    /home/eregon/code/rubygems/test/rubygems/test_gem_specification.rb:955:in `block in test_self_load_tainted'

2155 runs, 6732 assertions, 10 failures, 1 errors, 1 skips

@eregon eregon force-pushed the test-truffleruby branch 2 times, most recently from f7bf725 to 89a0b9b Compare March 9, 2020 20:04
ghost pushed a commit that referenced this pull request Mar 11, 2020
7275: Use the standard RUBY_ENGINE_VERSION instead of JRUBY_VERSION r=deivid-rodriguez a=eregon

* RUBY_ENGINE and RUBY_ENGINE_VERSION are defined on every modern Ruby.
* There is no such constant as TRUFFLERUBY_VERSION or RBX_VERSION.

This blocks #2797.
Also see #2864

Co-authored-by: Benoit Daloze <eregontp@gmail.com>
@eregon
Copy link
Contributor Author

eregon commented Apr 21, 2020

Latest run in CI:

  1) Failure:
TestGemRequire#test_realworld_default_gem [/home/runner/work/rubygems/rubygems/test/rubygems/test_require.rb:415]:
Expected "" to not be empty.

  2) Failure:
TestGemRequire#test_activate_via_require_respects_loaded_files [/home/runner/work/rubygems/rubygems/test/rubygems/test_require.rb:258]:
the benchmark stdlib should be recognized as already loaded

2170 runs, 6782 assertions, 2 failures, 0 errors, 0 skips

Just 2 failures left.

@deivid-rodriguez
Copy link
Member

Thanks so much for your work on this @eregon 👍.

@eregon eregon force-pushed the test-truffleruby branch 2 times, most recently from cb9e93d to 90fb194 Compare April 22, 2020 09:57
@eregon
Copy link
Contributor Author

eregon commented Apr 22, 2020

Thanks, it's in large part due to @bjfish too :)

It's now down to 2 failures, test_activate_via_require_respects_loaded_files was quite hard to understand so I clarified by adding some comments and improved the logic in that test.
And test_realworld_default_gem is a known TruffleRuby bug I'll take a look at.

eregon added 6 commits May 9, 2020 12:48
… tests

* Clearly require the benchmark stdlib instead of far away in test_case.rb
* When testing in the rubygems/rubygems repository, the previous code would
  move the lib/ dir at the end of $LOAD_PATH, which would cause to load
  a mix of lib/ RubyGems and in-stdlib-dir RubyGems, which blows up.
* But not enforce that error looks like
  /full/path/to/ruby/bin/ruby: No such file or directory
  but instead allow
  truffleruby: No such file or directory

A typical output for gem_make.out looks like:
current directory: /.../rubygems/tmp/test_rubygems_112388/gemhome/gems/a-2
/.../ruby-2.6.6/bin/ruby -I /.../rubygems/lib -r ./siteconf20200422-112388-nyrcy0.rb extconf.rb ''
/.../ruby-2.6.6/bin/ruby: No such file or directory -- extconf.rb (LoadError)
* Test that the correct version is loaded and that the default gem is
  not loaded at all.
* The --testing-rubygems flag is needed as otherwise TruffleRuby shows
  extra output when detecting it's using gem homes of other Rubies.
@eregon
Copy link
Contributor Author

eregon commented May 9, 2020

This is now passing, TruffleRuby now passes all RubyGems tests 🎉 🎉 🎉

This is thanks to @bjfish and my work which fixed many things in TruffleRuby and also spent a fair amount of time understanding what some of the tests try to do and why they failed, which took a long time (>1 year since #2700, working on this a little bit every while) to get everything passing.

https://github.com/rubygems/rubygems/runs/658849554?check_suite_focus=true

2180 runs, 6818 assertions, 0 failures, 0 errors, 0 skips

And that's with zero test excluded!

There are a few improvements to existing tests to be more reliable in this PR, as well as an added test for upgraded default gems as the existing test was not testing much.

Unfortunately these tests take a long time on TruffleRuby, I would think mostly because they use a lot of subprocesses and load quite a bit of code (+ of course tests are typically worst-case for a JIT, running a lot of different code just once). We're generally working on improving loading code speed in TruffleRuby so this should improve over time.
I don't think this should be much of an issue in practice though, this run takes ~23min, but each of the Bundler test suite runs takes ~15min, and there are many of them, and Windows Bundler tests take > 30min. So it shouldn't change much about total CI time and throughput.

@deivid-rodriguez @hsbt Could you review and merge?

@eregon eregon marked this pull request as ready for review May 9, 2020 11:06
* JRuby doesn't support multi-line -e.
@eregon eregon changed the title [WIP] Test TruffleRuby in CI Test TruffleRuby in CI May 9, 2020
* This is useful by showing the time for each command.
@hsbt hsbt self-assigned this May 9, 2020
@@ -12,16 +12,18 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
ruby: [ 2.3.8, 2.4.10, 2.5.8, 2.6.6, 2.7.1, jruby-9.2.11.1 ]
ruby: [ 2.3.8, 2.4.10, 2.5.8, 2.6.6, 2.7.1, jruby-9.2.11.1, truffleruby-head ]
Copy link
Member

Choose a reason for hiding this comment

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

Should we test with the stable version of truffleruby?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The latest release doesn't pass the tests, so it's not an option.

I'll make sure to run RubyGems tests in TruffleRuby's CI too (already the case, but some tests are excluded and no longer need to be).
TruffleRuby's master (and therefore truffleruby-head) is only updated if the entire TruffleRuby CI passes, so it should not cause failures.

Copy link
Member

Choose a reason for hiding this comment

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

I see. We can add the stable version of TruffleRuby after releasing from the current master version.

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

Successfully merging this pull request may close these issues.

None yet

5 participants