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

Require rails_helper before configuring RSpec #243

Merged
merged 2 commits into from
Mar 11, 2024
Merged

Conversation

Pacyfik
Copy link
Contributor

@Pacyfik Pacyfik commented Mar 7, 2024

Story

https://trello.com/c/iyN1GKYS/320-bug-rspec-knapsackpro-700-fails-with-the-dotenv-gem

Related

#242

Description

Version 7.0.0 introduced some fundamental changes, namely fetching, loading and running batches of specs after executing suite hooks, so that such hooks are only ran once, not before every batch. As a result, if rails_helper is only required in spec files, which is the RSpec default, instead of e.g. in .rspec, then some before(:suite) hooks, e.g. defined by gems, are registered after suite hooks had already been executed by the test suite. In comparison, RSpec loads all the spec files before executing before(:suite) hooks.

Changes

This PR conditionally adds --require rails_helper to cli arguments of KnapsackPro::Runners::Queue::RSpecRunner so that the Rails app (and its gems) can be loaded before executing before(:suite) hooks. That way all the before(:suite) hooks are registered properly and can be executed.

Checklist reminder

  • You follow the architecture outlined below for RSpec in Queue Mode, which is a work in progress (feel free to propose changes):
    • Pure: lib/knapsack_pro/pure/queue/rspec_pure.rb contains pure functions that are unit tested.
    • Extension: lib/knapsack_pro/extensions/rspec_extension.rb encapsulates calls to RSpec internals and is integration and e2e tested.
    • Runner: lib/knapsack_pro/runners/queue/rspec_runner.rb invokes the pure code and the extension to produce side effects, which are integration and e2e tested.

@Pacyfik Pacyfik added wip Work in progress RSpec labels Mar 7, 2024
@Pacyfik Pacyfik self-assigned this Mar 7, 2024
@Pacyfik Pacyfik force-pushed the require-rails-helper branch 2 times, most recently from 23f00af to a965543 Compare March 7, 2024 02:15
lib/knapsack_pro/version.rb Outdated Show resolved Hide resolved
@Pacyfik Pacyfik force-pushed the require-rails-helper branch from d2bebcc to 616cc4a Compare March 7, 2024 13:53
Copy link
Member

@ArturT ArturT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good direction. I added some comments.

We will need integration tests in spec/integration/runners/queue/rspec_runner_spec.rb. We want to cover case when the rails_helper.rb file exists and it has RSpec before(:suite), after(:suite) hooks defined and only a spec file in 2nd batch of tests requires rails_helper.

lib/knapsack_pro/runners/queue/rspec_runner.rb Outdated Show resolved Hide resolved
lib/knapsack_pro/adapters/rspec_adapter.rb Outdated Show resolved Hide resolved
@Pacyfik Pacyfik force-pushed the require-rails-helper branch from 616cc4a to 6d507bd Compare March 8, 2024 01:02
Copy link
Member

@ArturT ArturT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work!

spec/integration/runners/queue/rspec_runner_spec.rb Outdated Show resolved Hide resolved
spec/integration/runners/queue/rspec_runner_spec.rb Outdated Show resolved Hide resolved
@Pacyfik Pacyfik force-pushed the require-rails-helper branch from ec8e0e8 to ede4590 Compare March 11, 2024 14:43
@Pacyfik Pacyfik removed the wip Work in progress label Mar 11, 2024
@Pacyfik Pacyfik force-pushed the require-rails-helper branch from ede4590 to 10de75b Compare March 11, 2024 15:33
Co-Authored-By: Artur Trzop <arturtrzop@gmail.com>
@Pacyfik Pacyfik force-pushed the require-rails-helper branch from 10de75b to 80e389d Compare March 11, 2024 15:44
@Pacyfik Pacyfik merged commit 15101b8 into master Mar 11, 2024
45 checks passed
@Pacyfik Pacyfik deleted the require-rails-helper branch March 11, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants