Project

General

Profile

Feature #10682

Add "excludes" support to test suite, for alternative implementations and platforms

Added by headius (Charles Nutter) over 4 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
[ruby-core:67255]

Description

JRuby uses MRI's test suite as our primary compatibility suite. We would like to enhance the suite to support excluding tests.

Before the juggling of minitest versions in stdlib, JRuby was using minitest-excludes to exclude tests we knew didn't pass. The EXCLUDE_DIR env pointed at a dir with .rb files named after the test classes and containing "exclude" lines as seen here: https://github.com/jruby/jruby/blob/jruby-1_7/test/externals/ruby1.9/excludes/TestBignum.rb

This allowed us to maintain a "high water mark" for compatibility as MRI's suite evolved. It is largely the same feature as mspec/RubySpec's "tags".

Unfortunately minitest 5 does not support excludes, and MRI's suite has frozen a version of minitest 4 to use that is incompatible with the minitest-excludes library. In addition, many tests have returned to running with test/unit.

I provide here a patch for the exclude support we added to our copy of MRI's suite: https://gist.github.com/headius/4226cd94bbcf7b150e65

The only significant changes from minitest-excludes:

  • The env var is now "EXCLUDES". This is negotiable, but I don't know if enough people are using (or ever will use again) minitest-excludes, so the compatibility issue is moot.
  • There's no require needed to activate the excludes; if the env var is present, it will attempt to load them.

I'm willing to tune and improve this as necessary. JRuby master (9k, 9.0.0.0, 2.2-compatible) has been running MRI's tests this way for several months.

Associated revisions

Revision 1b7efc13
Added by nobu (Nobuyoshi Nakada) over 4 years ago

test/unit.rb: ExcludesOption

  • test/lib/test/unit.rb (ExcludesOption): add "excludes" support to test suite, for alternative implementations and platforms. [Feature #10682]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@49143 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 49143
Added by nobu (Nobuyoshi Nakada) over 4 years ago

test/unit.rb: ExcludesOption

  • test/lib/test/unit.rb (ExcludesOption): add "excludes" support to test suite, for alternative implementations and platforms. [Feature #10682]

Revision 49143
Added by nobu (Nobuyoshi Nakada) over 4 years ago

test/unit.rb: ExcludesOption

  • test/lib/test/unit.rb (ExcludesOption): add "excludes" support to test suite, for alternative implementations and platforms. [Feature #10682]

Revision 49143
Added by nobu (Nobuyoshi Nakada) over 4 years ago

test/unit.rb: ExcludesOption

  • test/lib/test/unit.rb (ExcludesOption): add "excludes" support to test suite, for alternative implementations and platforms. [Feature #10682]

Revision 49143
Added by nobu (Nobuyoshi Nakada) over 4 years ago

test/unit.rb: ExcludesOption

  • test/lib/test/unit.rb (ExcludesOption): add "excludes" support to test suite, for alternative implementations and platforms. [Feature #10682]

Revision 49143
Added by nobu (Nobuyoshi Nakada) over 4 years ago

test/unit.rb: ExcludesOption

  • test/lib/test/unit.rb (ExcludesOption): add "excludes" support to test suite, for alternative implementations and platforms. [Feature #10682]

History

Updated by headius (Charles Nutter) over 4 years ago

Pardon the leakchecker changes in that diff. leakchecker uses MRI-specific features we can't support (or simply don't support) in JRuby right now.

Updated by nobu (Nobuyoshi Nakada) over 4 years ago

I agree on the feature, and have a few questions.

  • Isn't the name exclude too generic?
  • Why do you define it as each singleton methods, not a singleton method of MiniTest::Unit::TestCase?
  • What part of LeakChecker doesn't work on JRuby?
  • Ditto for TracePointChecker, only TracePoint availablity?
  • Do you prefer alias-method-chain to prepend?

And I prefer to narrow the rescued region to only the File.read line,
and precede the rest code with an else.

Updated by headius (Charles Nutter) over 4 years ago

Nobuyoshi Nakada wrote:

I agree on the feature, and have a few questions.

  • Isn't the name exclude too generic?

Perhaps? I think the only conflict possible would be if there's a class-level method in some test that is also named "exclude". Seems unlikely? We also reduce the risk by only defining the exclude logic when the env is set.

  • Why do you define it as each singleton methods, not a singleton method of MiniTest::Unit::TestCase?

I guess because I wanted to keep the modifications as small as possible, and defining exclude permanently increases the risk of a name collision. However, these copies of test/unit and minitest are frozen and somewhat specific to this suite, so defining "exclude" permanently might not be so bad.

  • What part of LeakChecker doesn't work on JRuby?

ObjectSpace.each_object(IO) is one problem. However, we could run with ObjectSpace enabled and it might work ok.

The tempfile logic is also a problem, because we define our own native tempfile library and the monkey-patches here don't work right (e.g. we don't maintain a @count of open tempfiles in our impl).

  • Ditto for TracePointChecker, only TracePoint availablity?

TracePoint mostly works in JRuby, but not mostly enough for TracePointChecker. It should be temporary.

  • Do you prefer alias-method-chain to prepend?

I have no preference. I'm not using any 2.x features in this patch because when I updated our test suite we did not have most of those features implemented. It may be good to stick to a smaller subset of Ruby features in the plumbing for the test suite, so new implementations will have less to do to run it.

And I prefer to narrow the rescued region to only the File.read line,
and precede the rest code with an else.

Ok, I'll look into that.

Updated by headius (Charles Nutter) over 4 years ago

Patch updated: https://gist.github.com/headius/4226cd94bbcf7b150e65

I only made the rescue change requested by nobu. If there's a strong preference to use prepend or to add "exclude" once as a singleton method to some superclass, I can try it...but I'm happy with the way this works right now.

Some of the complexity is also due to some sub-suites using test/unit, so I'm not sure there's a single place we could define exclude...it would probably need to live in both test/lib/minitest and test/lib/test/unit.

Please let me know any remaining objections/concerns/suggestions by Monday, so I can commit this.

I will try to look into the leakchecker and tracepointchecker issues on JRuby, and file issues to improve them if the issues are because of implementation-specific behavior.

Updated by akr (Akira Tanaka) over 4 years ago

leafchecker should be used on CRuby.
But the patch comment out it so it is disabled regardless of ruby implementations.

Updated by nobu (Nobuyoshi Nakada) over 4 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

Applied in changeset r49143.


test/unit.rb: ExcludesOption

  • test/lib/test/unit.rb (ExcludesOption): add "excludes" support to test suite, for alternative implementations and platforms. [Feature #10682]

Also available in: Atom PDF