After Zeitwerk upgrade, specs don't autoload controller names

tmtrademarked commented 2 years ago

Please describe the issue

After upgrading from Gruf 2.14.1 to 2.15.0, specs started failing in our project due to a failure to autoload the RPC constants. It seems like there should either be a slight change to the library here (or the spec helper gruf-rspec), or perhaps a documented best practice?

How to Reproduce

We have a very simple spec for our RPC controller. The controller is defined in app/rpc/order_service_rpc_controller.rb, and the spec is spec/rpc/order_service_rpc_controller_spec.rb:

# frozen_string_literal: true

require 'rails_helper'

RSpec.describe OrderServiceRpcController do
  // Actual specs here
end

With the older version of gruf, this spec runs just fine. In the new version, Rails can't autoload the OrderServiceRpcController any longer because the app/rpc directory is not autoloaded in the test environment. This leads to:

NameError:
  uninitialized constant OrderServiceRpcController

What should happen?

It seems like there should be some documentation somewhere on the correct practice to test an RPC controller with the new autoloading framework enabled. We were able to work around this in our project, but I'm not convinced that adding explicit require statements in each spec is really the right solution.

splittingred commented 2 years ago

Hi @tmtrademarked - can you elaborate a bit on the version of Rails involved? Also Ruby version would be good as well. Any more context you can provide around the app setup would be good.

I'm not seeing this on various Rails 5/6/7 installations. Are you doing any specific types of autoloading elsewhere, as well?

Finally, if you're still stuck, does adding this to your spec/spec_helper.rb do the trick?

::Gruf.autoloaders.load!(controllers_path: Gruf.controllers_path)

tmtrademarked commented 2 years ago

Ah, yeah, I should have been more specific - apologies for that! This is on Ruby 3.0, Rails 6.1.6 using the Zeitwerk autoloader (ie config.load_defaults 6.1). I don't think we're doing anything else particularly fancy with autoloading anywhere in this application besides Gruf.

And yup, manually invoking the autoloaders in the spec_helper does seem to fix this problem, which makes sense. Is that a reasonable approach to fix this? If so, would this be worth documenting in the rspec helper, perhaps?

Thanks very much for taking a look!

splittingred commented 2 years ago

@tmtrademarked Let's go with that for now. I may take a look at manually adding that autoloading call to a pre-suite hook to gruf-rspec; though it's strange your system needed it and not others we use with similar setups. The call is idempotent, though, so you'd be safe adding it to your app for now. Needs more testing, for sure. Thanks!

splittingred commented 2 years ago

@tmtrademarked Probably relevant to you: https://github.com/bigcommerce/gruf-rspec/pull/17

tmtrademarked commented 2 years ago

Awesome - thanks @splittingred ! I'll give that a shot once it's released!

splittingred commented 2 years ago

@tmtrademarked https://rubygems.org/gems/gruf-rspec/versions/0.6.0

tmtrademarked commented 2 years ago

Confirmed that using 0.6.0 we no longer need to manually load the controllers. Thanks for the investigation here, and for the quick turn-around on a fix!

bigcommerce / gruf