Move specs out of dummy app v2 #100
Conversation
There was a problem hiding this comment.
Hmmm I'm not sure about this PR, @quorauk 🤔
This makes some changes to the gemfile, I think this is probably ok, but let me know if you want me to move that stuff into the gemspec instead
I definitely don't want to move the gems into the gemspec, I think that should just contain dependancies that the actual gem needs.
My main thought is that this a pretty big set of changes for allowing a contributor to run the tests from the top level in the app. Was going into the dummy app such a big problem?
I wonder if @defaye would like to take a look. I know he has spoken in the past about disliking how the the tests were in the dummy app.
Gemfile
Outdated
| # Reduces boot times through caching; required in config/boot.rb | ||
| gem 'bootsnap', '>= 1.1.0', require: false | ||
| gem 'sprockets-rails' | ||
| gem 'sqlite3' | ||
| gem 'rollbar', '~> 2.11', '>= 2.11.5' | ||
|
|
||
| group :development, :test do | ||
| # Call 'byebug' anywhere in the code to stop execution and get a debugger console | ||
| gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] | ||
| gem 'rspec-rails' | ||
| end | ||
|
|
||
| group :test do | ||
| gem 'capybara' |
There was a problem hiding this comment.
Can we move all these gems into the test group?
Also could you see if there is a way to include one gem file in another?
bin/setup
Outdated
| cd spec/dummy-app | ||
| bundle install | ||
| ./bin/rails db:setup | ||
| ./bin/rails assets:precompile | ||
| cd ../../ |
There was a problem hiding this comment.
This could definitely use a comment.
| You can also run `bin/console` for an interactive prompt that will allow you to experiment. | ||
| Run `./bin/setup` to install all gems and setup the dummy app. | ||
|
|
||
| You can then run the tests from the root of the app using `bundle exec rspec` |
There was a problem hiding this comment.
Can you make it clear that you need to run ./bin/setup before you can run any of the tests?
|
|
||
| gem 'turbo-rails' | ||
|
|
There was a problem hiding this comment.
Rails couldn't pick this up when its in the test group for some reason
| @@ -1,4 +1,4 @@ | |||
| ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__) | |||
| ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __dir__) | |||
There was a problem hiding this comment.
This allows the dummy app to use the gemfile from the gem instead of the one in the dummy app root directory
| require 'active_record' | ||
|
|
||
| ActiveRecord::Migration.verbose = false | ||
| ActiveRecord::Base.logger = Logger.new(nil) | ||
|
|
||
| if Rails.gem_version >= Gem::Version.new('6.0.0') | ||
| ActiveRecord::MigrationContext.new(File.expand_path('../dummy/db/migrate', __dir__), ActiveRecord::SchemaMigration).migrate | ||
| elsif Rails.gem_version >= Gem::Version.new('5.2.0') | ||
| ActiveRecord::MigrationContext.new(File.expand_path('../dummy/db/migrate', __dir__)).migrate | ||
| end | ||
|
|
||
| DatabaseCleaner[:active_record].strategy = :transaction | ||
| ORMInvalidRecordException = ActiveRecord::RecordInvalid |
There was a problem hiding this comment.
Performs all the database setup and teardown for running tests.
Removes the need to do cd spec/dummy-app; ./bin/rails db:setup on first run
Changes
Moves specs out of dummy app so they can be run without changing directory, and as you would expect from a typical ruby project
This makes some changes to the gemfile, I think this is probably ok, but let me know if you want me to move that stuff into the gemspec instead
@PatMiekina has made changes to update the dummy app to rails 7, I think this should allow that to happen without needing to rewrite the tests to the new app
There are additionally some changes to the github workflow that are a little gross to get migrations and gems loaded. Let me know if you have any alternative approaches
Ticket
https://github.com/orgs/pixielabs/projects/4?pane=issue&itemId=31864894
Before submitting have you:
After submitting remember to: