modified `request_specs` to fit API requests. #2046

exocode · 2018-12-07T09:34:33Z

Basic json header is added and could be customized later by the user (when using Authentication logic).
Usage:
generate scaffold v1::test name:string --api
Automagically knows if factory_bot or fabrication gem is installed and places their corresponding generators within the valid_attributes hash.
This behaviour can also be overrided by using --fabrication or --factory

In order to work properly, some methods were copied from the controller_spec.

Related: #2043

Basic json header is added and could be customized later by the user. For example when using Authentication logic. `generate scaffold v1::test name:string --api` automagically knows if `factory_bot` or `fabrication` gem is installed and places their corresponding generators within the `valid_attributes` hash. This behaviour can also be overrided by using `--fabrication` or `--factory` In order to work properly, some methods were copied from the controller_spec.

…mespaced fabricators built.

JonRowe

Good start! Some early feedback for you

JonRowe · 2018-12-07T10:21:12Z

lib/generators/rspec/scaffold/scaffold_generator.rb

@@ -33,7 +33,8 @@ def generate_controller_spec
          "#{controller_file_name}_controller_spec.rb"
        )
        if options[:api]
-          template 'api_controller_spec.rb', template_file
+          # skip generation of controller_spec. Specs are in request_spec if an API is scaffolded
+          # template 'api_controller_spec.rb', template_file


We don't commit commented out code, you will need a:

# For api templates we generate a request spec instead unless options[:api] template 'controller_spec.rb', template_file end

And remove the old file too.

JonRowe · 2018-12-07T10:22:25Z

lib/generators/rspec/integration/integration_generator.rb

@@ -10,12 +10,64 @@ class IntegrationGenerator < Base
                   :type => :boolean,
                   :default => true,
                   :desc => "Generate request specs"
+      class_option :api, :type => :boolean, :desc => "Creates request_spec for APIs, skip specs unnecessary for API-only apps"
+      class_option :fabrication, :type => :boolean, :desc => "Fill params with Fabricator model attributes"
+      class_option :factory, :type => :boolean, :desc => "Fill params with Factory Bot aka Factory Girl model attributes"


If this option is specific to factory bot then it should be called factorybot for consistency, also can you remove the aka Factory Girl reference please, most people know its changed and I'd rather we didn't drag up the debate.

JonRowe · 2018-12-07T10:23:56Z

lib/generators/rspec/integration/integration_generator.rb

+        template_file = File.join(
+            'spec/requests',
+            class_path,
+            "#{table_name}_requests_spec.rb"


We don't need requests twice.

JonRowe · 2018-12-07T10:25:49Z

lib/generators/rspec/integration/templates/api_request_spec.rb

+  # This should return the minimal set of attributes required to create a valid
+  # <%= class_name %>. As you add validations to <%= class_name %>, be sure to
+  # adjust the attributes here as well.
+  let(:valid_attributes) {


Multi line lets should be do/end.

JonRowe · 2018-12-07T10:25:53Z

lib/generators/rspec/integration/templates/api_request_spec.rb

+  # <%= class_name %>. As you add validations to <%= class_name %>, be sure to
+  # adjust the attributes here as well.
+  let(:valid_attributes) {
+<% if options[:factory] || Gem.loaded_specs.has_key?('factory_bot_rails') -%>


I would rather we make these configurations opt in only, rather than magically turn them on, what if Rubygems is not available (quite possible, we turn it off in our builds), also what if they have a different gem installed? (Just factory bot, or the older name for the gem :P)

JonRowe · 2018-12-07T10:26:25Z

lib/generators/rspec/integration/templates/api_request_spec.rb

+
+
+<% module_namespacing do -%>
+RSpec.describe <%= class_name.pluralize %>Controller, <%= type_metatag(:request) %> do


Not controller...

JonRowe · 2018-12-07T10:27:05Z

lib/generators/rspec/integration/templates/api_request_spec.rb

+    it "returns a success response" do
+      <%= file_name %> = <%= class_name %>.create! valid_attributes
+
+      get <%= ns_file_name.pluralize %>_path, params: {}, headers: valid_headers


When did Rails api get introduced? Just wondering if these are keyword args or a hash, if the latter should be hash rockets :/

I dont understand what you mean... :-/

Older versions of rails accepted a hash of options to their get/post/put etc methods, some time around Rails 4 this was changed to keyword arguments. If these are keyword arguments this is fine, but if this is a hash of arguments we tend to favour the hashrocket :key => value style due to older Ruby support. Did you copy these from the controller equivalent? If so thats fine as this will be the same level of support.

This needs resolving

This can now safely be ignored, you can use rails 5 syntax.

exocode · 2019-10-14T11:12:21Z

Can someone please overfly and tell me what exactly is missing or I have to change?
Rails API is a core feature that I use. I always have to delete the controller_spec where a request_spec is needed.

JonRowe

Hi @exocode, the basic premises of the review is that I'm happy to add this is it meets the following criteria:

All versions of Rails / Ruby that we support must be supported. That is the main crux of the issue right now.
That this is a configurable option for generation, so existing people wanting to generate controller specs still can, so you need to duplicate the templates rather than modify existing ones. In rspec-rails 4 this can be made the default.
You need to remove the support for 3rd party tools, that doesn't belong here.

JonRowe · 2019-10-14T11:45:26Z

lib/generators/rspec/integration/integration_generator.rb

@@ -10,12 +10,58 @@ class IntegrationGenerator < Base
                   :type => :boolean,
                   :default => true,
                   :desc => "Generate request specs"
+      class_option :api, :type => :boolean, :desc => "Creates request_spec for APIs, skip specs unnecessary for API-only apps"
+      class_option :fabrication, :type => :boolean, :desc => "Fill params with Fabricator model attributes"
+      class_option :factorybot, :type => :boolean, :desc => "Fill params with Factory Bot model attributes"


These are best left to outside gems, we don't have any other integration with extensions like this, so we can't include this here.

JonRowe · 2019-10-14T11:47:25Z

lib/generators/rspec/integration/integration_generator.rb

+      class_option :fabrication, :type => :boolean, :desc => "Fill params with Fabricator model attributes"
+      class_option :factorybot, :type => :boolean, :desc => "Fill params with Factory Bot model attributes"
+
+


Duplicate spacing

JonRowe · 2019-10-14T11:47:45Z

lib/generators/rspec/integration/integration_generator.rb

+          template 'request_spec.rb',
+                   File.join('spec/requests', class_path, "#{table_name}_spec.rb")
+        end
+


Excess whitespace and weird line formatting.

JonRowe · 2019-10-14T11:48:06Z

lib/generators/rspec/integration/integration_generator.rb

+
+      attr_reader :generator_args
+
+      # @todo refactor the following methods. They are also in the /lib/generators/rpsec/scaffold/scaffold_generator.rb file.


Either refactor these, or remove the todo. If this PR isn't complete it can't be merged.

JonRowe · 2019-10-14T11:48:43Z

lib/generators/rspec/integration/templates/api_request_spec.rb

-# expectations of assigns and templates rendered. These features have been
-# removed from Rails core in Rails 5, but can be added back in via the
-# `rails-controller-testing` gem.
+


You can't just remove existing functionality, if you wish to add more functionality thats fine but these files must remain as is, further comments assume you've done this and apply to a new file containing functionality.

lib/generators/rspec/integration/templates/api_request_spec.rb

pirj · 2020-01-08T20:53:55Z

@exocode are you interested in finishing this? This contribution would be very valuable.

exocode · 2020-01-09T07:02:35Z

@exocode are you interested in finishing this? This contribution would be very valuable.

I will continue that! At the moment I've to finish another project urgently. I will give it a try

pirj · 2020-03-13T09:10:35Z

@exocode Do you think this still applies after https://github.com/rspec/rspec-rails/pull/2288/files#diff-251cf12c9198d8742474434672773adeR1 being merged?

JonRowe · 2020-03-13T09:12:51Z

I'm going to close this for now as there is an api option to generate request specs when scaffolded and for controllers. Please try the new generators and open a new PR if you feel they can be improved!

exocode added 2 commits December 7, 2018 10:29

fixed naming convention from fabrication gem when namespaced / non na…

a0f4790

…mespaced fabricators built.

JonRowe requested changes Dec 7, 2018

View reviewed changes

exocode added 7 commits December 7, 2018 11:31

removed commented template generation

e5587fd

removed unused controller_spec for api

0d4bebd

renamed factory to factorybot

d345938

removed: We don't need requests twice.

4775094

do/end instead of braces

700bb41

removed automatic fabrication and factorybot creation

b1a46c3

renamed spec title

9cbe4d3

JonRowe requested changes Oct 14, 2019

View reviewed changes

klyonrad mentioned this pull request Jan 11, 2020

Generate request specs when scaffold generator is used #2262

Closed

6 tasks

JonRowe closed this Mar 13, 2020



		<% module_namespacing do -%>
		RSpec.describe <%= class_name.pluralize %>Controller, <%= type_metatag(:request) %> do

		class_option :fabrication, :type => :boolean, :desc => "Fill params with Fabricator model attributes"
		class_option :factorybot, :type => :boolean, :desc => "Fill params with Factory Bot model attributes"


		attr_reader :generator_args

		# @todo refactor the following methods. They are also in the /lib/generators/rpsec/scaffold/scaffold_generator.rb file.

Uh oh!

modified request_specs to fit API requests. #2046

modified request_specs to fit API requests. #2046

Uh oh!

Conversation

exocode commented Dec 7, 2018 • edited by benoittgt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonRowe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

exocode commented Oct 14, 2019

Uh oh!

JonRowe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pirj commented Jan 8, 2020

Uh oh!

exocode commented Jan 9, 2020

Uh oh!

pirj commented Mar 13, 2020

Uh oh!

JonRowe commented Mar 13, 2020

Uh oh!

Uh oh!

modified `request_specs` to fit API requests. #2046

modified `request_specs` to fit API requests. #2046

exocode commented Dec 7, 2018 •

edited by benoittgt

Loading