rspec / rspec-core

RSpec runner and formatters
http://rspec.info
MIT License
1.23k stars 765 forks source link

Shared contexts/examples files_organization / code_loading #3050

Closed DDKatch closed 1 year ago

DDKatch commented 1 year ago

I was quite surprised that I could not use shared contexts feature out of the box in my ruby project.

Finally I found a solution.

Here is my directories structure:

Screenshot 2023-10-23 at 23 35 14

Here is the necessary code in spec_helper.rb:

# load shared contexts
Dir['spec/support/**/*.rb'].each do |f|
  require File.expand_path(f)
end

My point is: why do I need to require files placed in pretty much conventional directory for core feature? Is there something I don't see?

My environment

JonRowe commented 1 year ago

:wave: Ruby itself has no auto requiring etc of any files by default, it places lib onto the load path and by convention gems start from lib/<their-name>.rb and require,autoload or load etc from there. Bundler requires gems by default (unless you tell it not to) so a lot of things appear to be inferred but they are actually explicitly loaded, RSpec follows this convention. Some frameworks (such as Rails, autoload files with more magic) but its not been something we do.

The pattern you mention is actually generated as part of a rspec-rails install but not the plain spec/spec_helper.rb, if you wanted to improve the generated spec helper to mention this pattern commented out as an example...

DDKatch commented 1 year ago

@JonRowe thanks for the quick reply.

It is clear that Ruby does not load every file in project directories by itself. This responsibility was taken by zeitwerk gem in the recent rails versions for example.

My concern about file loading comes from this sentence: "I believe that every core feature should work out of the box". For example one of the rails core features - MVC, so we have models, controllers, view directories. Imagine that files from controllers and views would be autoloaded, but files from models dir would have to be loaded manually with require. Looks strange at least. I have had similar confusion with shared contexts. Maybe my labeling of "shared contexts/examples" as the core feature is wrong.

There is a convention for rspec - load files with _spec.rb suffix. But "shared logic" is not a "spec" so it has to be loaded another way. I get it. However at the same time, I would love to have it explicitly mentioned in the documentation (so I will see that I can't fully rely on rspec autoload), not to mention to have it preloaded automatically without order conflicts (which may be resolved with already mentioned zeitwerk I think).

I'd love to update the documentation at least, if my reasons above make sense to you.

pirj commented 1 year ago

convention for rspec - load files with _spec.rb suffix

  1. We don’t load all, and don’t want to. You may have e.g. a custom rubocop cop in your code, and loading Rails-related specs/shared examples is wasteful.

  2. There is no single one-fits-all directory structure for shared examples. If ‘support’ was in the load_path, it would be an easier decision. Some prefer to put shared examples/contexts closer to their specs, and not to ‘shared’, and load them with ‘require_relative’.

  3. Auto-loading won’t work. We could do that. But I have no certainty we want to. Also, edge cases like ‘[“left”, “right”].each do |dir| RSpec.shared_context dir do’ - what file name you’ll put it to to auto-load?

A pr showing an example of how shared examples CAN be loaded is probably fine, but I would be opposed if it promotes the “one true way” of doing things. In any case, I would rely on @JonRowe opinion here.

JonRowe commented 1 year ago

I won't argue that shared examples should work out of the box but they do work out of the box, you can (and people do) use them in spec files themselves (they're actually scoped by example group) and you can put them in your spec_helper, in fact thats whats you're doing by requiring extra files you're breaking up the spec helper.

I agree it would be odd if a framework with an autoloading behaviour only loaded certain parts of it self and you had to autoload the rest, but thats not how I see RSpec here, the pattern for loading specs has a default spec/**/*_spec.rb (which is also a configurable option) which is what we use to find and load specs, you can also override this by passing explicit specs, so we don't actually autoload anything, we load a specific set of files and then follow Ruby semantics. You'll note you have to require spec_helper.rb too, its just normally in a .rspec or other option.

In theory theres nothing stopping you configuring adding and zeitwerk to load your support folder but thats not a responsibility RSpec wishes to take on.

DDKatch commented 1 year ago

@pirj @JonRowe thanks for the comments.

I have carefully read them. I have also read more about rspec in docs, in the help command output, and in source code files. Now I see the ground you stand on and to be honest I agree with your points. I agree that it is not the rspec responsibility to do a sophisticated loading of ruby files.

I will open a PR soon with a small README.md changes about how the shared contexts/examples may be loaded to be used in test cases.

DDKatch commented 1 year ago

@pirj @JonRowe please, check this out https://github.com/rspec/rspec-core/pull/3051