Switch out test-spec for rspec 2 for Ruby 1.9 compatability #62

Closed
wants to merge 2 commits into
from

6 participants

@mipearson

Getting Test::Spec & Test::Unit working under Ruby 1.9 was a dead end. It was much easier for me to bring in rspec.

MiniTest was considered but the matcher syntax already present is much closer to what RSpec uses.

Five tests fail for me in this PR, but they're the same five tests that fail in the origin/master, so I'm guessing that's not my fault :)

@rkh
Official Rack repositories member

+1

@chneukirchen
Official Rack repositories member
@raggi
Official Rack repositories member

I would strongly prefer bacon, for quite a few reasons. The non-personal reasons are however, that we can control it, which is important as it is likely that we will support a broader set of rubies for longer than rspec will, and I do not want to be encumbered with patching such a dependency set.

This kind of problem already occurred in rack when I let tenderlove commit changes that use the ruby stdlib for escaping. It's been a buggy, security flawed disaster for us, with much more complex backports, version and platform sensitivities.

Our users are everywhere, and use everything, and while the bulk of the Rails world does use RSpec, and I acknowledge that, it's also true that the bulk of that world use MRI 1.8 or JRuby.

@mipearson

I have no strong preference for rspec other than it's what I'm used to. I hadn't heard of bacon until mentioned today.

Also important, and not mentioned above, is consistency with the main rack repository, which uses bacon as its test framework.

If I get time today I'll rewrite this in bacon. Its format is closer to the original Test::Spec format anyway.

@mipearson

Okay. It looks like the most recent gem release of bacon, released four years ago, doesn't support nested contexts which are used heavily in the existing texts. Going by the README, the version in github does.

I'm hesitant to port the tests to depend on a version of bacon that hasn't been released yet.

I'd recommend that either a new version of bacon be released or this PR merged as is.

@mipearson

.. also at the risk of taking this off topic, which Ruby implementations does rspec not support? I'm curious now. Rubinius? MacRuby? Embedded implementations?

@raggi
Official Rack repositories member

Thanks!

@raggi
Official Rack repositories member

RSpec probably supports most current implementations, but it is very likely it'll drop support for older implementations before we do.

@chneukirchen
Official Rack repositories member

@mipearson I'll make a new bacon release before the end of the year, ok?

@mipearson
@chneukirchen
Official Rack repositories member

Bacon 1.2 released.

@jjb
jjb commented Nov 2, 2014

Not sure how folks feel about the issues discussed above anymore. Remarkably, this PR doesn't need a rebase, so it might be an okay thing to merge.

It would make sense to use minitest for rack-contrib tests, that would seem to address various concerns described above (because now it's in standard lib and there are gems for earlier versions).

Do any of the participants in this discussion from 2012 have any opinions?

@jjb
jjb commented Nov 2, 2014

Ah, I just now see that @mipearson did another PR porting the test suite to bacon #63/

Another massive PR that wasn't merged. A thankless labor of love for @mipearson!

@raggi
Official Rack repositories member
@raggi
Official Rack repositories member
@jjb
jjb commented Nov 2, 2014

i agree with that rationale

however bacon is not very popular and is also third-party

so it seems the best solution is to use something from ruby std lib

@raggi what do you think of my idea to use minitest?

@mpalmer

I use rspec for my own stuff (mostly through inertia), but I've used minitest and it does the job very nicely, and its presence in core is a distinct plus.

@mipearson

I haven't used rack-contrib for years. I wasn't happy about porting the tests to yet another NIH test framework.

I think you should convert it to minitest. I will be unfollowing this repository.

@raggi
Official Rack repositories member

@jjb bacon isn't third party to chris or I ;-)

sure, minitest is a much more maintainable dependency, although I think we'd still have problems with cross-MRI version support from 1.8 through 2.0. Specifically, minitest monkey rapes test/unit in some versions of MRI and therefore has mixed loaded states in some cases that lead to differing behavior.

@jjb
jjb commented Nov 4, 2014

any reason to not make some rack-contrib require 1.9 in the near future? i think all the minitest features we need will have the same api across 1.9 and above

@mpalmer

@jjb: There are an astonishingly large number of 1.8 users still out there -- I know every time I use a 1.9ism in lvmsync, for example, because I immediately get a couple of bug reports about it. On the other hand, I don't have a problem per se with requiring 1.9, if it makes our lives easier.

@jjb
jjb commented Nov 5, 2014

those users can continue to use the older version of rack-contrib. surely they aren't starting fresh projects under 1.8 and expecting a rich ecosystem of libraries available?

that said, maybe we should ask rack maintainers what they think — maybe since this is affiliated with the rack project, which is an extremely widely used project which I imagine will support 1.8 for some time, 1.8 compatibility is important.

@raggi
Official Rack repositories member

@jjb no, the minitest APIs (and compatibility with rake and similar toos) changes throughout the 1.9 series. maybe you won't notice though, it'll mostly be debian folks using their packages that get screwed.

@mpalmer mpalmer referenced this pull request Jun 14, 2015
Closed

Use minitest #102

@mpalmer

This PR has been superceded by #102.

@mpalmer mpalmer closed this Jun 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment