Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration specs #24

Merged
merged 19 commits into from
Apr 8, 2014
Merged

Integration specs #24

merged 19 commits into from
Apr 8, 2014

Conversation

rossta
Copy link
Contributor

@rossta rossta commented Apr 8, 2014

I felt it important to get integration testing added since I'm now using this gem in production. This is a rather large patch so it may require some discussion, including whether or not it's worth breaking into smaller pieces.

Summary of changes:

  • adds integration specs with capybara against a "dummy" rails app in the spec/
  • refactors unit specs
  • changes to Devise::TwoFactorAuthenticationController necessary to get passing specs
  • adds a success message to devise.en.yml
  • adds Travis-CI support with .travis.yml
  • changes to Gemfile to allow running specs against multiple versions of Rails
  • default rake task that runs all specs

Notes:

  • The gem doesn't appear to support Rails 3.1 out of the box, so you may consider changing the required Rails version in the gemspec. Rails 3.1 testing is omitted from .travis.yml. When running specs against Rails 3.1, I got the following error:
../two_factor_authentication/lib/two_factor_authentication/orm/active_record.rb:11:in `<top (required)>': uninitialized constant ActiveRecord::ConnectionAdapters::Table (NameError)`. 
  • The tests run only against the latest version of Devise. Future work could include running tests against earlier versions as well
  • It'd be cool to add a badge for Travis

Build Status

Houdini added a commit that referenced this pull request Apr 8, 2014
@Houdini Houdini merged commit 4d42705 into Houdini:master Apr 8, 2014
@rossta rossta deleted the spec_rails_app branch April 9, 2014 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants