From 9164f82eee16fe0ca102be888162fc64c2c223e5 Mon Sep 17 00:00:00 2001 From: LeFnord Date: Sun, 15 Jan 2017 23:46:22 +0100 Subject: [PATCH] increases code coverage (#570) - removbes dead code - adds changelog entry - corrects coveralls bagde --- .rspec | 1 - .travis.yml | 3 +- CHANGELOG.md | 1 + README.md | 2 +- lib/grape-swagger.rb | 88 +++++-------------- lib/grape-swagger/doc_methods.rb | 4 - lib/grape-swagger/doc_methods/parse_params.rb | 9 +- .../doc_methods/tag_name_description.rb | 23 +++-- lib/grape-swagger/endpoint.rb | 2 +- lib/grape-swagger/rake/oapi_tasks.rb | 6 +- spec/lib/endpoint_spec.rb | 43 ++++++++- spec/lib/extensions_spec.rb | 42 +++++++++ spec/lib/oapi_tasks_spec.rb | 15 +++- spec/lib/operation_id_spec.rb | 6 +- spec/lib/parse_params_spec.rb | 58 ++++++++++++ spec/lib/tag_name_description_spec.rb | 77 ++++++++++++++++ spec/lib/version_spec.rb | 26 ++++++ 17 files changed, 308 insertions(+), 98 deletions(-) create mode 100644 spec/lib/parse_params_spec.rb create mode 100644 spec/lib/tag_name_description_spec.rb create mode 100644 spec/lib/version_spec.rb diff --git a/.rspec b/.rspec index 8c18f1ab..4e1e0d2f 100644 --- a/.rspec +++ b/.rspec @@ -1,2 +1 @@ ---format documentation --color diff --git a/.travis.yml b/.travis.yml index 46e82fe1..c12653b6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,7 @@ after_success: - coveralls matrix: + fast_finish: true include: - rvm: 2.4.0 script: @@ -34,11 +35,9 @@ matrix: - rvm: 2.3.3 - rvm: 2.2.6 - rvm: ruby-head - - rvm: jruby-9.1.6.0 - rvm: jruby-head - rvm: rbx-2 allow_failures: - rvm: ruby-head - - rvm: jruby-9.1.6.0 - rvm: jruby-head - rvm: rbx-2 diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e82e004..3af37971 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * [#567](https://github.com/ruby-grape/grape-swagger/pull/567): Issue#566: removes markdown - [@LeFnord](https://github.com/LeFnord). * [#568](https://github.com/ruby-grape/grape-swagger/pull/568): Adds code coverage w/ coveralls - [@LeFnord](https://github.com/LeFnord). +* [#570](https://github.com/ruby-grape/grape-swagger/pull/570): Removes dead code -> increases code coverage - [@LeFnord](https://github.com/LeFnord). * Your contribution here. diff --git a/README.md b/README.md index 6ca30883..cc661670 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ [![Gem Version](https://badge.fury.io/rb/grape-swagger.svg)](http://badge.fury.io/rb/grape-swagger) [![Build Status](https://travis-ci.org/ruby-grape/grape-swagger.svg?branch=master)](https://travis-ci.org/ruby-grape/grape-swagger) -[![Coverage Status](https://coveralls.io/repos/github/ruby-grape/grape-swagger/badge.svg)](https://coveralls.io/github/ruby-grape/grape-swagger) +[![Coverage Status](https://coveralls.io/repos/github/ruby-grape/grape-swagger/badge.svg?branch=master)](https://coveralls.io/github/ruby-grape/grape-swagger?branch=master) [![Dependency Status](https://gemnasium.com/ruby-grape/grape-swagger.svg)](https://gemnasium.com/ruby-grape/grape-swagger) [![Code Climate](https://codeclimate.com/github/ruby-grape/grape-swagger.svg)](https://codeclimate.com/github/ruby-grape/grape-swagger) diff --git a/lib/grape-swagger.rb b/lib/grape-swagger.rb index 8f2b8928..a12d6422 100644 --- a/lib/grape-swagger.rb +++ b/lib/grape-swagger.rb @@ -76,11 +76,8 @@ def combine_routes(app, doc_klass) def combine_namespaces(app) app.endpoints.each do |endpoint| - ns = if endpoint.respond_to?(:namespace_stackable) - endpoint.namespace_stackable(:namespace).last - else - endpoint.settings.stack.last[:namespace] - end + ns = endpoint.namespace_stackable(:namespace).last + # use the full namespace here (not the latest level only) # and strip leading slash mount_path = (endpoint.namespace_stackable(:mount_path) || []).join('/') @@ -93,7 +90,7 @@ def combine_namespaces(app) def combine_namespace_routes(namespaces) # iterate over each single namespace - namespaces.each do |name, namespace| + namespaces.each do |name, _| # get the parent route for the namespace parent_route_name = extract_parent_route(name) parent_route = @target_class.combined_routes[parent_route_name] @@ -102,55 +99,28 @@ def combine_namespace_routes(namespaces) !route_path_start_with?(route, name) || !route_instance_variable_equals?(route, name) end - if namespace.options.key?(:swagger) && namespace.options[:swagger][:nested] == false - # Namespace shall appear as standalone resource, use specified name or use normalized path as name - identifier = if namespace.options[:swagger].key?(:name) - name.tr(' ', '-') - else - name.tr('_', '-').gsub(/\//, '_') - end - @target_class.combined_namespace_identifiers[identifier] = name - @target_class.combined_namespace_routes[identifier] = namespace_routes - - # # get all nested namespaces below the current namespace - sub_namespaces = standalone_sub_namespaces(name, namespaces) - sub_routes = sub_routes_from(parent_route, sub_namespaces) - @target_class.combined_namespace_routes[identifier].push(*sub_routes) - else - # default case when not explicitly specified or nested == true - standalone_namespaces = namespaces.reject do |_, ns| - !ns.options.key?(:swagger) || - !ns.options[:swagger].key?(:nested) || - ns.options[:swagger][:nested] != false - end - - parent_standalone_namespaces = standalone_namespaces.reject { |ns_name, _| !name.start_with?(ns_name) } - # add only to the main route - # if the namespace is not within any other namespace appearing as standalone resource - if parent_standalone_namespaces.empty? - # default option, append namespace methods to parent route - parent_route = @target_class.combined_namespace_routes.key?(parent_route_name) - @target_class.combined_namespace_routes[parent_route_name] = [] unless parent_route - @target_class.combined_namespace_routes[parent_route_name].push(*namespace_routes) - end + # default case when not explicitly specified or nested == true + standalone_namespaces = namespaces.reject do |_, ns| + !ns.options.key?(:swagger) || + !ns.options[:swagger].key?(:nested) || + ns.options[:swagger][:nested] != false + end + + parent_standalone_namespaces = standalone_namespaces.reject { |ns_name, _| !name.start_with?(ns_name) } + # add only to the main route + # if the namespace is not within any other namespace appearing as standalone resource + # rubocop:disable Style/Next + if parent_standalone_namespaces.empty? + # default option, append namespace methods to parent route + parent_route = @target_class.combined_namespace_routes.key?(parent_route_name) + @target_class.combined_namespace_routes[parent_route_name] = [] unless parent_route + @target_class.combined_namespace_routes[parent_route_name].push(*namespace_routes) end end end def extract_parent_route(name) - route_name = name.match(%r{^/?([^/]*).*$})[1] - return route_name unless route_name.include? ':' - name.match(/\/[a-z]+/)[0].delete('/') - end - - def sub_routes_from(parent_route, sub_namespaces) - sub_ns_paths = sub_namespaces.collect { |ns_name, _| ["/#{ns_name}", "/:version/#{ns_name}"] } - sub_routes = parent_route.reject do |route| - parent_namespace = route_instance_variable(route) - !sub_ns_paths.assoc(parent_namespace) && !sub_ns_paths.rassoc(parent_namespace) - end - - sub_routes + name.match(%r{^/?([^/]*).*$})[1] end def route_instance_variable(route) @@ -169,24 +139,6 @@ def route_path_start_with?(route, name) route.path.start_with?(route_prefix, route_versioned_prefix) end - def standalone_sub_namespaces(name, namespaces) - # assign all nested namespace routes to this resource, too - # (unless they are assigned to another standalone namespace themselves) - sub_namespaces = {} - # fetch all namespaces that are children of the current namespace - namespaces.each { |ns_name, ns| sub_namespaces[ns_name] = ns if ns_name.start_with?(name) && ns_name != name } - # remove the sub namespaces if they are assigned to another standalone namespace themselves - sub_namespaces.each do |sub_name, sub_ns| - # skip if sub_ns is standalone, too - next unless sub_ns.options.key?(:swagger) && sub_ns.options[:swagger][:nested] == false - # remove all namespaces that are nested below this standalone sub_ns - sub_namespaces.each do |sub_sub_name, _| - sub_namespaces.delete(sub_sub_name) if sub_sub_name.start_with?(sub_name) - end - end - sub_namespaces - end - def create_documentation_class Class.new(Grape::API) do extend GrapeSwagger::DocMethods diff --git a/lib/grape-swagger/doc_methods.rb b/lib/grape-swagger/doc_methods.rb index 1b147068..e3fe3362 100644 --- a/lib/grape-swagger/doc_methods.rb +++ b/lib/grape-swagger/doc_methods.rb @@ -14,10 +14,6 @@ module GrapeSwagger module DocMethods - def name - @@class_name - end - def hide_documentation_path @@hide_documentation_path end diff --git a/lib/grape-swagger/doc_methods/parse_params.rb b/lib/grape-swagger/doc_methods/parse_params.rb index c9f9e844..3a3c7ec2 100644 --- a/lib/grape-swagger/doc_methods/parse_params.rb +++ b/lib/grape-swagger/doc_methods/parse_params.rb @@ -99,15 +99,10 @@ def param_type(value_type) def parse_enum_or_range_values(values) case values + when Proc + parse_enum_or_range_values(values.call) when Range parse_range_values(values) if values.first.is_a?(Integer) - when Proc - values_result = values.call - if values_result.is_a?(Range) && values_result.first.is_a?(Integer) - parse_range_values(values_result) - else - { enum: values_result } - end else { enum: values } if values end diff --git a/lib/grape-swagger/doc_methods/tag_name_description.rb b/lib/grape-swagger/doc_methods/tag_name_description.rb index 08f33c65..03488f1e 100644 --- a/lib/grape-swagger/doc_methods/tag_name_description.rb +++ b/lib/grape-swagger/doc_methods/tag_name_description.rb @@ -4,14 +4,27 @@ class TagNameDescription class << self def build(paths) paths.values.each_with_object([]) do |path, memo| - path.values.first[:tags].each do |tag| - memo << { - name: tag, - description: "Operations about #{tag.pluralize}" - } + tags = path.values.first[:tags] + + case tags + when String + memo << build_memo(tags) + when Array + path.values.first[:tags].each do |tag| + memo << build_memo(tag) + end end end.uniq end + + private + + def build_memo(tag) + { + name: tag, + description: "Operations about #{tag.pluralize}" + } + end end end end diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 83737538..e9ca6ff3 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -8,7 +8,7 @@ def content_types_for(target_class) if content_types.empty? formats = [target_class.format, target_class.default_format].compact.uniq - formats = Grape::Formatter::Base.formatters({}).keys if formats.empty? + formats = Grape::Formatter.formatters({}).keys if formats.empty? content_types = Grape::ContentTypes::CONTENT_TYPES.select do |content_type, _mime_type| formats.include? content_type end.values diff --git a/lib/grape-swagger/rake/oapi_tasks.rb b/lib/grape-swagger/rake/oapi_tasks.rb index 62dfdf6b..96c463ed 100644 --- a/lib/grape-swagger/rake/oapi_tasks.rb +++ b/lib/grape-swagger/rake/oapi_tasks.rb @@ -35,9 +35,11 @@ def fetch store – save as JSON file, default: false (optional) resource - if given only for that it would be generated (optional)' task fetch: :environment do + # :nocov: make_request save_to_file? ? File.write(file, @oapi) : $stdout.print(@oapi) + # :nocov: end end @@ -47,6 +49,7 @@ def validate params (usage: key=value): resource - if given only for that it would be generated (optional)' task validate: :environment do + # :nocov: ENV['store'] = 'true' ::Rake::Task['oapi:fetch'].invoke exit if error? @@ -55,6 +58,7 @@ def validate $stdout.puts 'install swagger-cli with `npm install swagger-cli -g`' if output.nil? FileUtils.rm(file) + # :nocov: end end @@ -62,7 +66,7 @@ def validate # def make_request get url_for - last_response + @oapi = JSON.pretty_generate( JSON.parse( last_response.body, symolize_names: true diff --git a/spec/lib/endpoint_spec.rb b/spec/lib/endpoint_spec.rb index 9bd1b378..ea61d9c1 100644 --- a/spec/lib/endpoint_spec.rb +++ b/spec/lib/endpoint_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Grape::Endpoint do - subject { described_class.new(Grape::Util::InheritableSetting.new, path: '/', method: :get) } + subject do + described_class.new(Grape::Util::InheritableSetting.new, path: '/', method: :get) + end describe '#param_type_is_array?' do it 'returns true if the value passed represents an array' do @@ -15,4 +17,43 @@ expect(subject.send(:param_type_is_array?, '[String, Integer]')).to be_falsey end end + + describe '.content_types_for' do + describe 'defined on target_class' do + let(:own_json) { 'text/own-json' } + let(:own_xml) { 'text/own-xml' } + let(:content_types) do + { + own_json: own_json, + own_xml: own_xml + } + end + let(:target_class) { OpenStruct.new(content_types: content_types) } + + let(:object) { subject.content_types_for(target_class) } + specify do + expect(object).to eql [own_json, own_xml] + end + end + + describe 'not defined' do + describe 'format given' do + let(:format) { :json } + let(:target_class) { OpenStruct.new(format: format) } + let(:object) { subject.content_types_for(target_class) } + specify do + expect(object).to eql ['application/json'] + end + + describe 'format not given' do + let(:target_class) { OpenStruct.new } + let(:object) { subject.content_types_for(target_class) } + + specify do + expect(object).to eql %w(application/xml application/json text/plain) + end + end + end + end + end end diff --git a/spec/lib/extensions_spec.rb b/spec/lib/extensions_spec.rb index 08de73ff..dfd45c3e 100644 --- a/spec/lib/extensions_spec.rb +++ b/spec/lib/extensions_spec.rb @@ -1,6 +1,48 @@ require 'spec_helper' describe GrapeSwagger::DocMethods::Extensions do + describe '#find_definition' do + subject { described_class } + + let(:method) { :get } + let(:status) { 200 } + + before { allow(subject).to receive(:method).and_return(method) } + + describe 'no response for status' do + let(:path) { { get: { responses: {} } } } + + specify do + definition = subject.find_definition(status, path) + expect(definition).to be_nil + end + end + + describe 'response found' do + let(:model) { 'Item' } + + describe 'ref given' do + let(:path) do + { get: { responses: { 200 => { schema: { '$ref' => "#/definitions/#{model}" } } } } } + end + specify do + definition = subject.find_definition(status, path) + expect(definition).to eql model + end + end + + describe 'items given' do + let(:path) do + { get: { responses: { 200 => { schema: { 'items' => { '$ref' => "#/definitions/#{model}" } } } } } } + end + specify do + definition = subject.find_definition(status, path) + expect(definition).to eql model + end + end + end + end + describe '#extended? and extension' do subject { described_class } describe 'return false (default)' do diff --git a/spec/lib/oapi_tasks_spec.rb b/spec/lib/oapi_tasks_spec.rb index 9636c38b..81da329a 100644 --- a/spec/lib/oapi_tasks_spec.rb +++ b/spec/lib/oapi_tasks_spec.rb @@ -5,11 +5,11 @@ item = Class.new(Grape::API) do version 'v1', using: :path - resource :item do + namespace :item do get '/' end - resource :otherItem do + namespace :otherItem do get '/' end end @@ -100,6 +100,17 @@ end end end + + describe 'call it' do + before do + subject.send(:make_request) + end + specify do + expect(subject).to respond_to :oapi + expect(subject.oapi).to be_a String + expect(subject.oapi).not_to be_empty + end + end end describe '#file' do diff --git a/spec/lib/operation_id_spec.rb b/spec/lib/operation_id_spec.rb index 12dcd50c..bd1565b7 100644 --- a/spec/lib/operation_id_spec.rb +++ b/spec/lib/operation_id_spec.rb @@ -7,11 +7,7 @@ specify { expect(subject).to respond_to :build } describe 'build' do - if defined?(Grape::VERSION) && Gem::Version.new(::Grape::VERSION) < Gem::Version.new('0.16.0') - let(:route) { Grape::Route.new(method: method) } - else - let(:route) { Grape::Router::Route.new(method, '/path', requirements: {}) } - end + let(:route) { Grape::Router::Route.new(method, '/path', requirements: {}) } describe 'GET' do let(:method) { 'GET' } diff --git a/spec/lib/parse_params_spec.rb b/spec/lib/parse_params_spec.rb new file mode 100644 index 00000000..1d5bf645 --- /dev/null +++ b/spec/lib/parse_params_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe GrapeSwagger::DocMethods::ParseParams do + subject { described_class } + let(:start_value) { -5 } + let(:end_value) { 5 } + + describe '#parse_range_values' do + specify do + parsed_range = subject.send(:parse_range_values, start_value..end_value) + expect(parsed_range).to eql(minimum: start_value, maximum: end_value) + end + end + + describe '#parse_enum_or_range_values' do + describe 'value as Range' do + describe 'first Integer' do + specify do + parsed_range = subject.send(:parse_enum_or_range_values, start_value..end_value) + expect(parsed_range).to eql(minimum: start_value, maximum: end_value) + end + end + + describe 'first String' do + specify do + parsed_range = subject.send(:parse_enum_or_range_values, 'a'..'z') + expect(parsed_range).to be_nil + end + end + end + + describe 'value as Proc' do + describe 'as Range' do + let(:values) { proc { start_value..end_value } } + specify do + parsed_range = subject.send(:parse_enum_or_range_values, values) + expect(parsed_range).to eql(minimum: start_value, maximum: end_value) + end + end + + describe 'as Array' do + let(:values) { proc { %w(a b c) } } + specify do + parsed_range = subject.send(:parse_enum_or_range_values, values) + expect(parsed_range).to eql(enum: %w(a b c)) + end + end + end + + describe 'values as Array -> enums' do + let(:values) { %w(a b c) } + specify do + parsed_range = subject.send(:parse_enum_or_range_values, values) + expect(parsed_range).to eql(enum: %w(a b c)) + end + end + end +end diff --git a/spec/lib/tag_name_description_spec.rb b/spec/lib/tag_name_description_spec.rb new file mode 100644 index 00000000..5e3dc5e3 --- /dev/null +++ b/spec/lib/tag_name_description_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe GrapeSwagger::DocMethods::TagNameDescription do + describe '#build_memo' do + let(:tag) { 'some_string' } + subject { described_class.send(:build_memo, tag) } + + specify do + expect(subject.keys).to eql [:name, :description] + expect(subject).to eql( + name: tag, + description: "Operations about #{tag.pluralize}" + ) + end + end + + describe '#build' do + subject { described_class.build(paths) } + describe 'empty paths' do + let(:paths) { {} } + specify do + expect(subject).to eql([]) + end + end + + describe 'paths given' do + describe 'uniq as String' do + let(:paths) do + { key_1: { post: { tags: 'tags_given' } } } + end + + specify do + expect(subject).to eql [{ name: 'tags_given', description: 'Operations about tags_givens' }] + end + end + + describe 'uniq as Array' do + let(:paths) do + { key_1: { post: { tags: ['tags_given'] } } } + end + + specify do + expect(subject).to eql [{ name: 'tags_given', description: 'Operations about tags_givens' }] + end + end + + describe 'multiple' do + describe 'uniq key' do + let(:paths) do + { + key_1: { post: { tags: %w(tags_given another_tag_given) } } + } + end + + specify do + expect(subject).to eql [ + { name: 'tags_given', description: 'Operations about tags_givens' }, + { name: 'another_tag_given', description: 'Operations about another_tag_givens' } + ] + end + end + describe 'under different keys' do + let(:paths) do + { + key_1: { post: { tags: ['tags_given'] } }, + key_2: { post: { tags: ['tags_given'] } } + } + end + + specify do + expect(subject).to eql [{ name: 'tags_given', description: 'Operations about tags_givens' }] + end + end + end + end + end +end diff --git a/spec/lib/version_spec.rb b/spec/lib/version_spec.rb new file mode 100644 index 00000000..04e49541 --- /dev/null +++ b/spec/lib/version_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe GrapeSwagger::DocMethods::Version do + let(:route) { OpenStruct.new(version: version) } + subject { described_class.get(route) } + + describe 'grape 0.16.2 version' do + let(:version) { '[:v1, :v2]' } + it { is_expected.to be_a Array } + it { is_expected.to eql [:v1, :v2] } + end + + describe 'newer grape versions' do + describe 'as String' do + let(:version) { 'v1' } + it { is_expected.to be_a String } + it { is_expected.to eql 'v1' } + end + + describe 'as Array' do + let(:version) { [:v1, :v2] } + it { is_expected.to be_a Array } + it { is_expected.to eql [:v1, :v2] } + end + end +end