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

RSpec controller tests fail with v11 of the shopify app #879

Closed
Jay-Plumb opened this issue Jan 30, 2020 · 7 comments
Closed

RSpec controller tests fail with v11 of the shopify app #879

Jay-Plumb opened this issue Jan 30, 2020 · 7 comments
Labels

Comments

@Jay-Plumb
Copy link

Hey team,

Until v10 of the shopify app, it was possible to stub out the sessions controller within our RSpec tests such as:

# spec/support/controller_session.rb

module ControllerSession
  def shopify_controller_session(shop)
    @request.session[:shopify] = shop.id
    @request.session[:shopify_domain] = shop.shopify_domain
  end
end

From there, we could define a before action within our controller tests which would prevent our test from redirecting due to not having an activated session:

 # within any controller_spec.rb file
  before do
    @shop = create(:installed_shop)
    shopify_controller_session(@shop)
  end

However, since v11 of the shopify app gem, I found the above no longer worked and after looking at how /lib/shopify_app/controller_concerns/login_protection.rb operated, I fixed the test suite by doing the following:

# spec/support/controller_session.rb

module ControllerSession
  def shopify_controller_session(shop)
    stubbed_session = ShopifyAPI::Session.new(domain: shop.shopify_domain, token: shop.shopify_token, api_version: shop.api_version)
    allow(ShopifyApp::SessionRepository).to receive(:retrieve).and_return(stubbed_session)
    @request.session[:shopify] = shop.id
    @request.session[:shopify_domain] = shop.shopify_domain
  end
end

All I am doing is stubbing out the retrieve call to ShopifyApp::SessionRepository and returning a stubbed version and allows the session to be activated.

Now, the reason why I wanted to open an issue is to help share my solution with others. I have had comments in the past with how to still test controllers when inheriting from ShopifyApp::AuthenticatedController. I think it could be really useful to have the above added to the README.md to help others that use RSpec for their testsuite. I am more than happy to make the change but I will wait for feedback from the team first. Equally, if anyone has a better solution to the above then do share and we can take it from there!

@tiffanytse
Copy link
Contributor

Thanks @Jay-Plumb this is a duplicate; please follow along here: #874

@tanema
Copy link
Contributor

tanema commented Jan 30, 2020

@Jay-Plumb Thank you for sharing your workaround. It does seem like this is not a duplicate (As I told tiffany it might be, it was my fault she closed it! Sorry) I just want to clarify. Your tests stubbing changed to add versioning? Is that what change? Or do you know exactly why you need to change it this way?

I think I will reopen this because it would be a good idea to add test helpers for this gem to make it easier for people to test their code. This way these sort of issues need to be opened

@tanema tanema reopened this Jan 30, 2020
@Jay-Plumb
Copy link
Author

Jay-Plumb commented Jan 30, 2020

Hey @tanema, I attempted the solution within #874 but I can confirm that the approach did not work for me. I believe the reason for the change was in relation to switching from:

# config/initializers/shopify_app.rb
ShopifyApp.configure do |config|
  ...
  config.session_repository = Shop
end

to:

# config/initializers/shopify_app.rb
ShopifyApp.configure do |config|
  ...
  config.session_repository = 'ShopifyApp::InMemorySessionStore'
end

but I am not 100% sure if that is the cause. However, I can definitely confirm that on v10 the old approach worked but I hit into issues from v11.4.0 onwards.

Edit: For my use-case I actually want to keep the former (config.session_repository = Shop). What I listed might still be useful for future developers but it might not be as useful as I initially thought. Anyway, feel free to keep the "issue" open in relation to providing test helpers in the future and equally I am okay if you decide the close the issue.

Thanks!

@mattkc7
Copy link

mattkc7 commented May 22, 2020

I'm stuck on this, however the proposed workaround isn't working for me :(

@ristooi
Copy link

ristooi commented Aug 6, 2020

I also encountered this issue with shopify_app 14.0.0, proposed solution did not work for me.
This worked:

def login_shop(shop)
    stubbed_session = ShopifyAPI::Session.new(domain: shop.shopify_domain, token: shop.shopify_token, api_version: shop.api_version)
    expect(ShopifyApp::SessionRepository).to receive(:retrieve_shop_session).and_return(stubbed_session)
    @request.session[:shop_id] = shop.id
end

@github-actions
Copy link

This issue is stale because it has been open for 2 years. It will be closed if no further action occurs in 14 days.

@github-actions github-actions bot added the stale label Sep 11, 2022
@github-actions
Copy link

We are closing this issue because it has been inactive for a few months.
This probably means that it is not reproducible or it has been fixed in a newer version.
If it’s an enhancement and hasn’t been taken on since it was submitted, then it seems other issues have taken priority.

If you still encounter this issue with the latest stable version, please reopen using the issue template. You can also contribute directly by submitting a pull request– see the CONTRIBUTING.md file for guidelines

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants