JQuery-rails gem spec removal

Hi there,

I don’t know if this is the right place to discuss such a topic. I’ve suggested to remove jquery-rails spec folder, since it hasn’t got any tests, they are not adapted to RSpec2 and not even to latest rails. They are one year old and it contains only one empty test.

I have an application that runs all tests on all engines, being jquery-rails one of them. I have to explicitly skip this engine since tests are not passing, and given that they aren’t useful at all I didn’t even work on fixing them.

https://github.com/rails/jquery-rails/issues/55

What do you think ?

And sorry in the first place if this isn’t the place to discuss this. If this isn’t the place I’d be glad to be redirected to the right place :slight_smile:

Best regards,

Rafael Fernández López.

What exactly was the discussion you wanted to have? Seems like a pretty simple change. Or were you simply asking to have it pulled in (in which case, it looks like it just was)?

You are correct that the spec folder wasn’t being used, it was just legacy code. If anyone is curious (and I suppose I’ll update the jquery-rails readme with this info), it’s currently tested in two ways.

  1. The jquery-ujs adapter that jquery-rails bundles is tested with qunit.

  2. And jquery-rails itself has an integration test suite within a basic rails app, which allows me to catch any potential issues pertaining to changes in the rails api. For example, if links started being generated with data-ajax=true instead of data-remote, this would catch it and let me know the gem needs to be updated. Or, an example that actually did happen. when the behavior with the csrf token changed at one point.

#1 can be found within the rails/jquery-ujs project. For #2, I’ve been meaning to pull this into the jquery-rails repo, but at the moment, it’s still under my personal repo: https://github.com/JangoSteve/Rails-jQuery-Demo

– Steve Schwartz

The fix has already been merged in:
https://github.com/rails/jquery-rails/pull/56

Correct. Like I said, it’s already been pulled in. It sounded like he wanted to discuss it though, so I’m trying to make sure I didn’t miss anything.

– Steve Schwartz

Correct. Like I said, it's already been pulled in. It sounded like he wanted
to discuss it though, so I'm trying to make sure I didn't miss anything.

Whoops! I didn't see your email before I sent mine. Roger.

Hello Steve,

Sorry for the late reply. I wasn't looking for a conversation in particular, just asking first, if this was the right place to talk about it (I wasn't really sure), and second, if it was there for some reason I was missing.

So yes, it was basically to ask. Afterwards is when I submitted the pull request and got pulled in really fast.

Thanks for the detailed explanation, those tests look like necessary and seem great.

Best regards,
Rafael Fernández López.