ruby_bug guards

[eregon]

Should we prefer MSpec tags to ruby_bug guards? All other implementations use tags, that is files listing excluded specs to ensure all good specs are still good and not run known failing specs.

MRI situation is a bit different as it is the reference implementation. But I am wondering if ruby_bug is really worth it. Using tags instead would mean we would have tag files in some repository (MRI repo under spec/tags, in respective branches?), probably one set of tags per released version to allow different versions to fail different specs (either by using branches or an external tag repo with different directories for 2.0, 2.1, 2.2 and trunk).

Of course, in an ideal world, there would be no failing spec. But this is reality, and backporting a fix to an already released version might not always be possible or a good decision. So, some old released versions must at least exclude some specs. For trunk, there really should be close to zero excluded spec. But some bugs are not easy to fix and might need some temporary exclusion until they eventually get fixed.

I know somes pros and cons, but I would like to hear your opinion, particularly @nurse, @hsbt and @headius.

Pros of ruby_bug:

• Also a version guard, so no need of "per-version tag set".
• Documents the issue number, which might be useful for more information about the bug or spec (the issue number could be added in a tag comment, but it is less visible). Value of the issue number to other implementations is difficult to say, but normally the spec itself should be enough.
• States clearly that some version of MRI does not conform to that spec, so using MRI to test behavior might be slightly less surprising. I doubt this is really significant in practice.
• Can test older versions and still pass all the specs (like in 4486d676). I do not know if this is desired at all, since I think very few people run RubySpec with old released versions. I think it is reasonable to simply require to run with an older RubySpec for old versions, or accept there might be failures.

Pros of tags:

• Fair to other implementations: they do not have to pass specs guarded by ruby_bug, while MRI fails them. It is quite rare people depend on the specified behavior if even MRI does not conform to it. A good ruby_version_is guard is also sometimes a good replacement for ruby_bug since it means the behavior is different or unspecified before some given version (for instance see 23095b78 which now uses a version guard).
• Existing tools to add/remove/verify them (mspec tag)
• Less intrusive than ruby_bug in that it does not modify the spec source.
• Clearer output of how much specs are tagged (this is new in MSpec).
• Clearer list of "known MRI bugs": the set of tag files.

[headius]

I'm on the side of not using ruby_bug, mostly because it favors one implemention over all others, forcing the others to run specs that are skipped in MRI. We have argued against its use since the beginning.

Addressing the pros of ruby_bug:

• Version guards already exist, and using them would allow other impls to set their version and know they're not running specs that don't apply. As it is now, a ruby_bug guard doesn't prevent the spec from running on other implementations no matter what because it's ignored everywhere but MRI. Every ruby_bug, regardless of relevance, has to be tagged by other impls. Even more confusing...once a bug is fixed and becomes a regular version guard, it may stop running in other impls when it was running before.
• Issue number is definitely valuable, but could simply be in a comment or in the specdoc description.
• If MRI does not conform to a spec, then it is a bug in MRI. When we have a bug in JRuby and don't pass a spec, we commit a tag to our repository. MRI should do the same.
• You could still run older versions of MRI against newer RubySpec using tags. And if something fails and isn't tagged, then it should either have a version guard (newer behavior) or it should be tagged and fixed (bug in old behavior).

ruby_bug also allows MRI to "cheat" on pass/fail rates, since they're not counted and not run...but other impls DO have to run them, and they count against us. Having a guard that only works on one impl and gives that impl a false positive just seems like a bad idea.

[eregon]

Most of the current ruby_bug guards are also for versions below 2.0, in which case we can safely remove them I think.

[anthonycrumley]

I am against using ruby_bug as well. I may be off base with my next statement because I am new to all this and my thoughts are not seasoned with the experience you guys have.

It seems to me that tags should not be used by MRI either.

RubySpec is currently a descriptive specification rather than a prescriptive one. It should accurately describe the way MRI behaves regardless of whether that behavior is desirable or not. Ruby is what MRI does. RubySpec should describe the actual behavior of MRI. Other implementations should use tags to show where they deviate from MRI, whether that deviation is considered positive or negative.

I think that only version guards should be used to differentiate the variations of behavior between MRI versions.

Some of these so called bugs are the way Ruby behaves for years and across multiple major and minor versions. A good example of this is the behavior of ensure in Fiber#resume.

https://github.com/ruby/rubyspec/blob/master/core/fiber/resume_spec.rb#L17-L35

This ruby_bug guard has been moved from 1.9.3 to 2.0 to 2.1 to 2.2 to 3.0. Yet the actual behavior is not even being tested because one day it may get fixed. If the actual behavior changes along the way, no tests will fail because the test for this functionality is in a ruby_bug or would be tagged not to run.

Seems it would be better to have a test to verify that the actual behavior remains consistent over time than it is to treat it as an exception that stays in place for years.

[eregon]

@anthonycrumley Yes, ruby_bug should only be used for actual bugs, like segfaults or behavior reported on the issue tracker and recognized as bugs. I agree the Fiber#resume ensure example is a bad spec. Behavior is not like that and there is no concrete plan to fix it, so leave it unspecified or specify current behavior. Having some way to specify accepted alternative behaviors might be useful when you think about differences such as the value Float::MIN. Of course one might argue it should be unspecified or only its existence but not the value, etc. Either way, I'm all for more uniformity between implementations in the specs.

[nurse]

I'm ok to eliminate ruby_bug after someone contribute MSpec tags for Ruby 2.0+.

[eregon]

I'll start by removing ruby_bug for versions < 2.0. @nurse Is it OK to contribute these tags to the MRI repo (in spec/tags for instance), in each concerned branch (trunk, ruby_2_2, ruby_2_1, ruby_2_0_0)? Or do you want an alternative?

[eregon]

There are only 2 ruby_bug left, which should be removed or replaced with appropriate version guards once we get a reply on the bug tracker. Therefore, I'll close this and consider ruby_bug deprecated. (ruby_version_is is often the better alternative if it is not fixed on older versions)