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

[Truffle] Add jt test mri command. #2819

Merged
merged 1 commit into from Apr 14, 2015
Merged

Conversation

bjfish
Copy link
Contributor

@bjfish bjfish commented Apr 10, 2015

This is just an idea to add a command to run mri tests.

The big args array was derived from https://travis-ci.org/jruby/jruby/jobs/57843253

@chrisseaton
Copy link
Contributor

This is definitely what we want to do next, but I've got quite a few comments on how we want to do this:

  • I'd like it to be under the jt test command, so it should be jt test mri.
  • I'd like it to be able to specify a directory or a single file, so I can do jt test mri test/mri/ruby/test_whileuntil.rb or jt test mri test/mri/ruby/test_whileuntil.rb
  • You're re-using JRuby's excludes there, which doesn't make any sense - can you create a full set of excludes for Truffle under test/truffle/excludes (may need to talk to @headius about how to do that)
  • You might want to modify jt tag, jt untag, and jt tag all to work with these excludes if they see a MRI test path rather than a RubySpec path
  • Do these tests actually work for us? Last time I checked we didn't run test/unit very well - if we do now that's excellent news
  • When it works and we have everything that doesn't pass excluded, we should add it to Travis

This patch would still be useful with just the first two points addressed.

@chrisseaton chrisseaton added this to the truffle-dev milestone Apr 10, 2015
@bjfish
Copy link
Contributor Author

bjfish commented Apr 10, 2015

@chrisseaton The first two points work now, jt test mri and jt test mri test/mri/ruby/test_whileuntil.rb
(I tested the command without -X+T).

I've ran a little bit of test/unit successfully. The first failure we'll run into here is:

/Users/brandonfish/Documents/jruby-mine/test/mri/lib/minitest/unit.rb:1389:in `(singleton-def)': undefined method `method_added' for #<Class:0x114> (NoMethodError)
    from /Users/brandonfish/Documents/jruby-mine/test/mri/lib/minitest/unit.rb:1388:in `TestCase'
    from /Users/brandonfish/Documents/jruby-mine/test/mri/lib/minitest/unit.rb:1242:in `Unit'
    from /Users/brandonfish/Documents/jruby-mine/test/mri/lib/minitest/unit.rb:746:in `MiniTest'
    from /Users/brandonfish/Documents/jruby-mine/test/mri/lib/minitest/unit.rb:12:in `<main>'
    from /Users/brandonfish/Documents/jruby-mine/test/mri/lib/test/unit.rb:5:in `<main>'
    from test/mri/runner.rb:7:in `<main>'

@bjfish bjfish changed the title [Truffle] Add jt mri test command. [Truffle] Add jt test mri command. Apr 10, 2015
@chrisseaton
Copy link
Contributor

Great - I'll merge after pre2 is out.

chrisseaton added a commit that referenced this pull request Apr 14, 2015
[Truffle] Add jt test mri command.
@chrisseaton chrisseaton merged commit bb9dbb7 into jruby:master Apr 14, 2015
scanf/test_scanfblocks.rb socket/test_basicsocket.rb socket/test_nonblock.rb socket/test_socket.rb stringio/test_stringio.rb strscan/test_stringscanner.rb
thread/test_queue.rb zlib/test_zlib.rb ruby/enc/test_big5.rb ruby/enc/test_cp949.rb ruby/enc/test_emoji.rb ruby/enc/test_euc_jp.rb ruby/enc/test_euc_kr.rb
ruby/enc/test_euc_tw.rb ruby/enc/test_gb18030.rb ruby/enc/test_gbk.rb ruby/enc/test_iso_8859.rb ruby/enc/test_koi8.rb ruby/enc/test_shift_jis.rb ruby/enc/test_utf16.rb
ruby/enc/test_utf32.rb ruby/enc/test_windows_1251.rb]
Copy link
Member

Choose a reason for hiding this comment

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

Large literals is almost never a good idea. Especially in a script file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes what we really want to do is add support for running jt test ruby/**/test_.rb or something like that.

Copy link
Member

Choose a reason for hiding this comment

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

Or do a proper Dir.glob or have a config file ala MSpec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eregon I think I tracked down how this is generated

./bin/jruby -X+T -e 'mri_test_files = File.readlines("test/mri.index").grep(/^[^#]\w+/).map(&:chomp).join(" ");puts mri_test_files'

Copy link
Member

Choose a reason for hiding this comment

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

Sweet, that might be a good replacement (we should have our own index at some point).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eregon BTW there are some workarounds required to apply in this patch if you want to try to use this command: https://gist.github.com/bjfish/1be84623da4a746f4d16

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I saw that :)
Patching jruby-core files is not an option, so either we should have shims (for Process.waitall for instance) or we should implement the subset needed for the functionality.

@enebo enebo added this to the Non-Release milestone Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants