Feature #10682: Add "excludes" support to test suite, for alternative implementations and platforms

• Status: Closed
• Assignee: -
• Target version: 2.2.0

[headius (Charles Nutter)]

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.

[headius (Charles Nutter)]

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.

[nobu (Nobuyoshi Nakada)]

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.

[headius (Charles Nutter)]

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.

[headius (Charles Nutter)]

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.

[akr (Akira Tanaka)]

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

[nobu (Nobuyoshi Nakada)]

• 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]