fixed most rubocop todos, increased coverage

Author: andi

.rubocop_todo.yml

@@ -6,79 +6,24 @@
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 4
-# This cop supports safe autocorrection (--autocorrect).
-# Configuration parameters: Max, AllowHeredoc, AllowURI, AllowQualifiedName, URISchemes, AllowRBSInlineAnnotation, AllowCopDirectives, AllowedPatterns, SplitStrings.
-# URISchemes: http, https
-Layout/LineLength:
-  Exclude:
-    - 'generators/navigation_config/templates/config/navigation.rb'
-    - 'spec/simple_navigation/helpers_spec.rb'
-
-# Offense count: 1
-# Configuration parameters: MaxRangeSize.
-Lint/MissingCopEnableDirective:
-  Exclude:
-    - 'spec/simple_navigation/configuration_spec.rb'
-
-# Offense count: 11
-# Configuration parameters: EnforcedStyle, CheckMethodNames, CheckSymbols, AllowedIdentifiers, AllowedPatterns.
-# SupportedStyles: snake_case, normalcase, non_integer
-# AllowedIdentifiers: TLS1_1, TLS1_2, capture3, iso8601, rfc1123_date, rfc822, rfc2822, rfc3339, x86_64
-Naming/VariableNumber:
-  Exclude:
-    - 'generators/navigation_config/templates/config/navigation.rb'
-    - 'spec/fake_app/config/navigation.rb'
-    - 'spec/simple_navigation/item_container_spec.rb'
-
-# Offense count: 122
-# Configuration parameters: Prefixes, AllowedPatterns.
-# Prefixes: when, with, without
-RSpec/ContextWording:
-  Exclude:
-    - 'spec/simple_navigation/adapters/rails_spec.rb'
-    - 'spec/simple_navigation/config_file_finder_spec.rb'
-    - 'spec/simple_navigation/configuration_spec.rb'
-    - 'spec/simple_navigation/helpers_spec.rb'
-    - 'spec/simple_navigation/item_adapter_spec.rb'
-    - 'spec/simple_navigation/item_container_spec.rb'
-    - 'spec/simple_navigation/item_spec.rb'
-    - 'spec/simple_navigation/items_provider_spec.rb'
-    - 'spec/simple_navigation/renderer/base_spec.rb'
-    - 'spec/simple_navigation/renderer/breadcrumbs_spec.rb'
-    - 'spec/simple_navigation/renderer/text_spec.rb'
-    - 'spec/simple_navigation_spec.rb'
-
-# Offense count: 4
-# Configuration parameters: Max, AllowedIdentifiers, AllowedPatterns.
-RSpec/IndexedLet:
-  Exclude:
-    - 'spec/simple_navigation/item_container_spec.rb'
-
 # Offense count: 46
 # Configuration parameters: .
 # SupportedStyles: have_received, receive
 RSpec/MessageSpies:
   EnforcedStyle: receive
 
-# Offense count: 38
-# Configuration parameters: EnforcedStyle, IgnoreSharedExamples.
-# SupportedStyles: always, named_only
-RSpec/NamedSubject:
-  Exclude:
-    - 'spec/simple_navigation_spec.rb'
-
 # Offense count: 24
 RSpec/SubjectStub:
   Exclude:
     - 'spec/simple_navigation/item_container_spec.rb'
     - 'spec/simple_navigation/renderer/base_spec.rb'
     - 'spec/simple_navigation_spec.rb'
 
-# Offense count: 65
+# Offense count: 69
 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
 RSpec/VerifiedDoubles:
   Exclude:
+    - 'spec/simple_navigation/adapters/nanoc_spec.rb'
     - 'spec/simple_navigation/adapters/padrino_spec.rb'
     - 'spec/simple_navigation/adapters/rails_spec.rb'
     - 'spec/simple_navigation/adapters/sinatra_spec.rb'

generators/navigation_config/templates/config/navigation.rb

@@ -35,16 +35,19 @@
   # navigation.ignore_query_params_on_auto_highlight = true
   # navigation.ignore_anchors_on_auto_highlight = true
 
-  # If this option is set to true, all item names will be considered as safe (passed through html_safe). Defaults to false.
+  # If this option is set to true, all item names will be considered as safe
+  # (passed through html_safe). Defaults to false.
   # navigation.consider_item_names_as_safe = false
 
   # Define the primary navigation
   navigation.items do |primary|
     # Add an item to the primary navigation. The following params apply:
     # key - a symbol which uniquely defines your navigation item in the scope of the primary_navigation
     # name - will be displayed in the rendered navigation. This can also be a call to your I18n-framework.
-    # url - the address that the generated item links to. You can also use url_helpers (named routes, restful routes helper, url_for etc.)
-    # options - can be used to specify attributes that will be included in the rendered navigation item (e.g. id, class etc.)
+    # url - the address that the generated item links to. You can also use url_helpers
+    #       (named routes, restful routes helper, url_for etc.)
+    # options - can be used to specify attributes that will be included in the rendered
+    #           navigation item (e.g. id, class etc.)
     #           some special options that can be set:
     #           :html - Specifies html attributes that will be included in the rendered navigation item
     #           :if - Specifies a proc to call to determine if the item should
@@ -58,19 +61,19 @@
     #                            when the item should be highlighted, you can set a regexp which is matched
     #                            against the current URI.  You may also use a proc, or the symbol <tt>:subpath</tt>.
     #
-    primary.item :key_1, 'name', url, options
+    primary.item :key1, 'name', url, options
 
     # Add an item which has a sub navigation (same params, but with block)
-    primary.item :key_2, 'name', url, options do |sub_nav|
+    primary.item :key2, 'name', url, options do |sub_nav|
       # Add an item to the sub navigation (same params again)
-      sub_nav.item :key_2_1, 'name', url, options
+      sub_nav.item :key21, 'name', url, options
     end
 
     # You can also specify a condition-proc that needs to be fullfilled to display an item.
     # Conditions are part of the options. They are evaluated in the context of the views,
     # thus you can use all the methods and vars you have available in the views.
-    primary.item :key_3, 'Admin', url, html: { class: 'special' }, if: -> { current_user.admin? }
-    primary.item :key_4, 'Account', url, unless: -> { logged_in? }
+    primary.item :key3, 'Admin', url, html: { class: 'special' }, if: -> { current_user.admin? }
+    primary.item :key4, 'Account', url, unless: -> { logged_in? }
 
     # you can also specify html attributes to attach to this particular level
     # works for all levels of the menu

lib/simple_navigation.rb

@@ -73,11 +73,13 @@ def set_env(root, environment)
     # Returns the current framework in which the plugin is running.
     def framework
       return :rails if defined?(Rails)
+      # :nocov:
       return :padrino if defined?(Padrino)
       return :sinatra if defined?(Sinatra)
       return :nanoc if defined?(Nanoc3)
 
       raise 'simple_navigation currently only works for Rails, Sinatra and Padrino apps'
+      # :nocov:
     end
 
     # Loads the adapter for the current framework

lib/simple_navigation/adapters/nanoc.rb

@@ -2,6 +2,9 @@
 
 module SimpleNavigation
   module Adapters
+    # @deprecated This adapter is for Nanoc3 (circa 2011-2012) and is no longer actively maintained.
+    #   Nanoc3 is obsolete and has been replaced by Nanoc 4+. This adapter is kept for
+    #   backward compatibility but should not be used for new projects.
     class Nanoc < Base
       class << self
         def register(root)

spec/fake_app/config/navigation.rb

@@ -2,7 +2,7 @@
 
 SimpleNavigation::Configuration.run do |navigation|
   navigation.items do |nav|
-    nav.item :item_1, 'Item 1', '/item_1', html: { class: 'item_1' }, link_html: { id: 'link_1' }
-    nav.item :item_2, 'Item 2', '/item_2', html: { class: 'item_2' }, link_html: { id: 'link_2' }
+    nav.item :item1, 'Item 1', '/item_1', html: { class: 'item_1' }, link_html: { id: 'link_1' }
+    nav.item :item2, 'Item 2', '/item_2', html: { class: 'item_2' }, link_html: { id: 'link_2' }
   end
 end

spec/simple_navigation/adapters/padrino_spec.rb

@@ -6,6 +6,29 @@
   let(:context) { double(:context, request: request) }
   let(:request) { double(:request) }
 
+  describe '.register' do
+    let(:padrino_module) { Module.new }
+    let(:padrino_application) { Module.new }
+
+    before do
+      stub_const('Padrino', padrino_module)
+      stub_const('Padrino::Application', padrino_application)
+      allow(padrino_module).to receive_messages(root: '/padrino/root', env: 'production')
+      allow(SimpleNavigation).to receive(:set_env)
+      allow(padrino_application).to receive(:send)
+    end
+
+    it 'calls SimpleNavigation.set_env with Padrino root and env' do
+      expect(SimpleNavigation).to receive(:set_env).with('/padrino/root', 'production')
+      described_class.register(nil)
+    end
+
+    it 'includes SimpleNavigation::Helpers in Padrino::Application' do
+      expect(padrino_application).to receive(:send).with(:helpers, SimpleNavigation::Helpers)
+      described_class.register(nil)
+    end
+  end
+
   describe '#link_to' do
     it 'delegates to context' do
       expect(context).to receive(:link_to).with('name', 'url', { my_option: true })

spec/simple_navigation/adapters/rails_spec.rb

@@ -79,15 +79,15 @@
     end
 
     context "when the controller's template is stored as view_context (Rails3)" do
-      context 'and the template is set' do
+      context 'when the template is set' do
         before { allow(controller).to receive_messages(view_context: template) }
 
         it "sets the adapter's template accordingly" do
           expect(adapter.template).to be template
         end
       end
 
-      context 'and the template is not set' do
+      context 'when the template is not set' do
         before { allow(controller).to receive_messages(view_context: nil) }
 
         it "sets the adapter's template to nil" do
@@ -101,15 +101,15 @@
     context "when the adapter's request is set" do
       before { allow(adapter).to receive_messages(request: request) }
 
-      context 'and request.fullpath is defined' do
+      context 'when request.fullpath is defined' do
         let(:request) { double(:request, fullpath: '/fullpath') }
 
         it "sets the adapter's request_uri to the request.fullpath" do
           expect(adapter.request_uri).to eq '/fullpath'
         end
       end
 
-      context 'and request.fullpath is not defined' do
+      context 'when request.fullpath is not defined' do
         let(:request) { double(:request, request_uri: '/request_uri') }
 
         before { allow(adapter).to receive_messages(request: request) }
@@ -153,15 +153,15 @@
     context "when the adapter's controller is set" do
       before { adapter.instance_variable_set(:@controller, controller) }
 
-      context "and the adapter's template is set" do
+      context "when the adapter's template is set" do
         before { adapter.instance_variable_set(:@template, template) }
 
         it "sets the adapter's context_for_eval to the template" do
           expect(adapter.context_for_eval).to be template
         end
       end
 
-      context "and the adapter's template is not set" do
+      context "when the adapter's template is not set" do
         before { adapter.instance_variable_set(:@template, nil) }
 
         it "sets the adapter's context_for_eval to the controller" do
@@ -173,15 +173,15 @@
     context "when the adapter's controller is not set" do
       before { adapter.instance_variable_set(:@controller, nil) }
 
-      context "and the adapter's template is set" do
+      context "when the adapter's template is set" do
         before { adapter.instance_variable_set(:@template, template) }
 
         it "sets the adapter's context_for_eval to the template" do
           expect(adapter.context_for_eval).to be template
         end
       end
 
-      context "and the adapter's template is not set" do
+      context "when the adapter's template is not set" do
         before { adapter.instance_variable_set(:@template, nil) }
 
         it 'raises an exception' do

spec/simple_navigation/adapters/sinatra_spec.rb

@@ -7,6 +7,19 @@
 
   before { allow(context).to receive_messages(request: request) }
 
+  describe '.register' do
+    let(:sinatra_app) { double(:app, root: '/sinatra/root', environment: 'test') }
+
+    before do
+      allow(SimpleNavigation).to receive(:set_env)
+    end
+
+    it 'calls SimpleNavigation.set_env with app root and environment' do
+      expect(SimpleNavigation).to receive(:set_env).with('/sinatra/root', 'test')
+      described_class.register(sinatra_app)
+    end
+  end
+
   describe '#context_for_eval' do
     context "when adapter's context is not set" do
       it 'raises an exception' do

spec/simple_navigation/config_file_finder_spec.rb

@@ -14,15 +14,15 @@
     context 'when the context is :default' do
       let(:context) { :default }
 
-      context 'and a navigation.rb file is found in one of the paths' do
+      context 'when a navigation.rb file is found in one of the paths' do
         before { FileUtils.touch('/path/one/navigation.rb') }
 
         it 'returns its full path' do
           expect(finder.find(context)).to eq '/path/one/navigation.rb'
         end
       end
 
-      context 'and no navigation.rb file is found in the paths' do
+      context 'when no navigation.rb file is found in the paths' do
         it 'raises an exception' do
           expect do
             finder.find(context)
@@ -34,15 +34,15 @@
     context 'when the context is :other' do
       let(:context) { :other }
 
-      context 'and a other_navigation.rb file is found in one of the paths' do
+      context 'when a other_navigation.rb file is found in one of the paths' do
         before { FileUtils.touch('/path/two/other_navigation.rb') }
 
         it 'returns its full path' do
           expect(finder.find(context)).to eq '/path/two/other_navigation.rb'
         end
       end
 
-      context 'and no other_navigation.rb file is found in the paths' do
+      context 'when no other_navigation.rb file is found in the paths' do
         it 'raise an exception' do
           expect do
             finder.find(context)

spec/simple_navigation/configuration_spec.rb

@@ -86,7 +86,7 @@
     before { allow(SimpleNavigation::ItemContainer).to receive_messages(new: container) }
 
     context 'when a block is given' do
-      context 'and items_provider is specified' do
+      context 'when items_provider is specified' do
         let(:provider) { double(:provider) }
 
         it 'raises an exception' do
@@ -95,6 +95,7 @@
             config.items(provider) do
             end
           end.to raise_error(RuntimeError, 'please specify either items_provider or block, but not both')
+          # rubocop:enable Lint/EmptyBlock
         end
       end
 
@@ -116,7 +117,7 @@
     end
 
     context 'when no block is given' do
-      context 'and items_provider is specified' do
+      context 'when items_provider is specified' do
         let(:external_provider) { double(:external_provider) }
         let(:items) { double(:items) }
         let(:items_provider) { double(:items_provider, items: items) }