Setup appraisal

[krzysiek1507]

Setup Appraisal so we can see if everything works on supported Rails versions.

[MaicolBen]

Awesome!! thanks

[krzysiek1507]

I'll take a look at failing Rails 4.2 migrations later.

[krzysiek1507]

Now I know, why v0.1.43 doesn't work in my project on Rails 4. ;P The gem doesn't work on Rails 4.

[krzysiek1507]

I would propose to split this PR into 2:

1. setup appraisal for Rails > 5
2. setup appraisal for Rails 4.2 and fix specs

[MaicolBen]

Sounds good! You can remove rails 4 appraisal from this PR. But it should work in Rails 4

[krzysiek1507]

@MaicolBen removed! I need this gem to work on Rails 4.2.

Do you know if there is a way to make Rails 5 get/post/patch/delete work with Rails 4.2? The have changed the signature of those methods.
Rails >= 5

get '/demo/members_only_group', params: {}, headers: @resource_auth_headers

Rails 4

get '/demo/members_only_group', {}, @resource_auth_headers

[krzysiek1507]

module Rails
  module Controller
    module Testing
      module Integration
        %w[get post patch put head delete get_via_redirect post_via_redirect].each do |method|
          define_method(method) do |path, **args|
            if Rails::VERSION::MAJOR >= 5
              super path, args
            else
              super path, args[:params], args[:headers]
            end
          end
        end
      end
    end
  end
end

[MaicolBen]

@krzysiek1507 Does it work? it looks great!

[krzysiek1507]

@MaicolBen it works but I need to fix some more specs. Could you please merge this PR? I'll create a new one with Rails 4.

[MaicolBen]

We try to have 2 approvals in each PR, adding more reviewers

[Evan-M]

@krzysiek1507 Approved and merged! Looking forward to the next PR with the Rails 4 appraisal. Thanks!

[Evan-M]

Hey, this merge is causing failing tests in travis...

Mocha::NotInitializedError:         Mocha::NotInitializedError: Mocha methods cannot be used outside the context of a test

Not sure why the merged code fails on the master branch, but the tests all pass fine for this PR?

UPDATE:
Looks like an additional change to the .travis.yml is needed... (See: thoughtbot/appraisal#25).

[krzysiek1507]

@Evan-M mocha 1.5.0 was released about merge time. 😄They have changed something.

[Evan-M]

Oh haha. Of course they did... that explains why the original Build #1184 starting reporting failures about halfway through the build jobs, whereas all the build jobs fail when I restarted #1184 just now.

[krzysiek1507]

I haven't used mocha but I can take a look at this tomorrow.

[Evan-M]

@krzysiek1507: I don't think the failure is specific to your code. In fact, Travis Build #1185 (which is for #1004) is experiencing the same failure from Mocha.

Looking over the code they added to the new release of Mocha 1.5.0, any attempt to use Mocha outside the context of a test or example will result in a Mocha::NotInitializedError being raised.

UPDATE: I created a new issue (#1137) for the Mocha failure.

[krzysiek1507]

@Evan-M I know that the problem is not specific to my code because I didn't change any line of code here. mocha notes say that they fix something so probably we need to adjust our tests to fixed code.

[Evan-M]

Hey @krzysiek1507: I know that I already merged this PR, and I just added PR #1139 (which resolves #1137), but I'm curious as to why the *.lock files for the Appraisal-created gemfiles have not been added to version control?

Specifically, these files:

/gemfiles/rails_5_0.gemfile.lock
/gemfiles/rails_5_1.gemfile.lock

The standard Gemfile.lock is version controlled, so shouldn't the gemfiles/*.lock files be as well? Is there a good reason to omit those particular files?

[krzysiek1507]

Hi @Evan-M. The question is: why do we want to control .lock files?

Answer 1: We want gem versions to be the same everywhere.
In this case shouldn't we lock minor/patch versions in Gemfile/gemspec?

Answer 2: We want to test using specified versions of gems.
Do we want to test using e.g. 2 years old gem version instead of newest?

For me, Gemfile and gemspec are files which describe the world and .lock files keep the generated state at some point.

[Evan-M]

@krzysiek1507: That is indeed a great way to think of it!

I've wondered for a while why there isn't more specificity of major/minor/patch versions in the Gemfile/gemspec. It seems like in lieu of more specificity in that regard, it is particularly important to have the *.lock files versioned, so that tagged releases of devise_token_auth have the same dependencies for everyone. That said, I believe that you're not really supposed to do that for a Gem.

Therefore, I think I kinda misphrased my question...

Unlike an application, it is best practice for a Gem to not include the *.lock files in version control (See Yehuda Katz's blog post on the matter). Instead, the ranges of versions that satisfy the dependencies should be specified in the gemspec, and the Gemfile should generally contain the Rubygems source and the gemspec line.

There is an additional comment in the Gemfile that suggests any dependencies that are still in development should go in the Gemfile, but be moved to the gemspec prior to release:

devise_token_auth/Gemfile

Lines 8 to 11 in 7df8960

	# Declare any dependencies that are still in development here instead of in
	# your gemspec. These might include edge Rails or gems from your path or
	# Git. Remember to move these dependencies to your gemspec before releasing
	# your gem to rubygems.org.

To be fair, I really only brought the issue of *.lock files up because of #1137... It seems like the gemspec ought to have a specific version of Mocha as a development_dependency. Without the specification, this PR had it's travis build run against one version of Mocha, and then after merging to master, the travis build ran against the new release of Mocha instead.

[krzysiek1507]

It seems like in lieu of more specificity in that regard, it is particularly important to have the *.lock files versioned, so that tagged releases of devise_token_auth have the same dependencies for everyone.

When you install a gem, bundler doesn't use Gemfile.lock nor Gemfile from this gem. One need Gemfile and Gemfile.lock in gem only for development and test.

There are 3 ways to solve this problem:

1. Remove *.lock, specify versions in gemspec/Gemfile.
2. Remove *.lock and use the newest versions of gems.
3. Add missing *.lock files but it will be hell to update deps in the future.