Skip to content

Comments

Test PE Feat/styling spec#3

Open
edthamm wants to merge 30 commits intomasterfrom
feat/styling-spec
Open

Test PE Feat/styling spec#3
edthamm wants to merge 30 commits intomasterfrom
feat/styling-spec

Conversation

@edthamm
Copy link
Owner

@edthamm edthamm commented Aug 19, 2018

Description

This PR updates some configuration files and fixes style issues in the specs.

This is the first of two related PR's the next one will be along in the coming days
and will do the same for everything in lib. I.e. assuming you want/like this kind
of work to be done there or at all.
I decided to start with the specs because they are often not as 'clever' as the production
code while providing a reasonably large codebase to discuss style issues.
Also I did not want to change everything at once because refactoring tests and production
code at the same time usually ends up breaking something.

This PR is also meant to serve as a starting point for discussion the style this project
wants.

Updated configuration:

  • Rubocop dependency upped to 0.54.0 == current version on hound
  • Update rubocop settings files to match new version
  • Set Style/StringLiterals to single-quote > Seems to be the current default
  • Set Layout/DotPosition to leading > Seems to be the current default
  • Set Metrics/LineLength to 125 > Not the default but less annoying
  • Set CI timeout to 45 seconds on JRuby

Used rubocops auto-correct as far as possible.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code cleanup

Notes to the reviewer

This is quite a huge PR.

The commits are prefaced with:

Label Description
Settings Change and adapt settings
Auto correct Fix style violations using rubocop auto-correct
Correct Fix style violation manually
Fix Fix an error or failing testcase after adaptations

Depending on what is changed in the commit.

I noticed that rubocop 0.54.0 does not play nice with the configuration files on my setup so I made the configuration explicit in the Guardfile.

Also at one point I thought that ruby < 2.3 is not supported anymore so I dropped
support for that. After I noticed my error I undid those changes.

How Has This Been Tested?

  • Run all existing tests locally in ruby 2.2.9, 2.5.0 and jruby 9.1.15.0
  • Run all existing tests in CI
  • Run bundle exec rubocop -c .rubocop.yml --rails spec

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

edthamm added 28 commits August 19, 2018 08:48
Update rubocop to match hound

Adapt hound style to new rubocop version

Add style config file for specs

Exclude vendor/**/*

Default for rubocop is to overwrite arrays. This happens here and leads to uncomfortable usage of rubocop locally.

Update target version to minimum supported version

Autoupdate rubocop settings using mry
If anyone is still using an 80 character long terminal to edit files on a regular basis,
contact me.
Also exclude specs from blocklength and erroneously drop a version from CI ->
fixed later

Explicitly enable Metrics/LineLength cop

Set dot position to leading

Set Style/StringLiterals to single-quote

This is the default value of the cop in newer versions of rubocop and it is an easy fix

Exclude spec from Metrics/BlockLength

DSLs and block length restraints usually do not work well together

Drop unsupported version from CI

Use single quotes in gemfile
Get rid of rubocop warnings

Remove superfluous config file
Version is actually still supported

Revert "Drop unsupported version from CI"

This reverts commit 93dd1e1.

Fix 2.2.9 incompatibility

This creates a performance hit but as this code is only test code, I deem this acceptable
expect { Guard.pause(:invalid) }.
to raise_error(ArgumentError, "invalid mode: :invalid")
expect { Guard.pause(:invalid) }
.to raise_error(ArgumentError, "invalid mode: :invalid")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

expect { Guard.pause(:invalid) }.
to raise_error(ArgumentError, "invalid mode: :invalid")
expect { Guard.pause(:invalid) }
.to raise_error(ArgumentError, "invalid mode: :invalid")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

expect(generator).to receive(:initialize_template).with("foo").
and_raise(Guard::Guardfile::Generator::NoSuchPlugin, "foo")
expect(generator).to receive(:initialize_template).with("foo")
.and_raise(Guard::Guardfile::Generator::NoSuchPlugin, "foo")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

before do
expect(generator).to receive(:initialize_template).with("foo").
and_raise(Guard::Guardfile::Generator::NoSuchPlugin, "foo")
expect(generator).to receive(:initialize_template).with("foo")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

expect(Guard::UI).to receive(:error).
with("No plugins found in Guardfile, please add at least one.")
expect(Guard::UI).to receive(:error)
.with("No plugins found in Guardfile, please add at least one.")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


it "returns a substituted single file specified within the action" do
expect(matched(%w(lib/foo.rb))).to eq ["spec/foo_spec.rb"]
expect(matched(%w[lib/foo.rb])).to eq ["spec/foo_spec.rb"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


it "returns nothing if the action returns empty string" do
expect(matched(%w(uptime.rb))).to eq [""]
expect(matched(%w[uptime.rb])).to eq [""]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


it "returns nothing if the action response is empty string" do
expect(matched(%w(blank.rb))).to eq [""]
expect(matched(%w[blank.rb])).to eq [""]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

expect(matched(%w(spec_helper.rb array.rb))).
to eq ["spec", %w(foo bar)]
expect(matched(%w[spec_helper.rb array.rb]))
.to eq ["spec", %w[foo bar]]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


it "returns multiple files specified within the action" do
expect(matched(%w(hash.rb))).to eq [{ foo: "bar" }]
expect(matched(%w[hash.rb])).to eq [{ foo: "bar" }]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

NOOP commit because hound hangs.

allow(File).to receive(:exist?).with("Gemfile").
and_return(gemfile_present)
allow(File).to receive(:exist?).with("Gemfile")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

before do
allow(config).to receive(:exist?).with(Pathname("Gemfile")).
and_return(false)
allow(config).to receive(:exist?).with(Pathname("Gemfile"))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

before do
allow(config).to receive(:exist?).with(Pathname("Gemfile")).
and_return(true)
allow(config).to receive(:exist?).with(Pathname("Gemfile"))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


allow(config).to receive(:exist?).with(Pathname("Gemfile")).
and_return(false)
allow(config).to receive(:exist?).with(Pathname("Gemfile"))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

allow(config).to receive(:exist?).
with(Pathname("/my/project/Gemfile")).and_return(false)
allow(config).to receive(:exist?)
.with(Pathname("/my/project/Gemfile")).and_return(false)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

expect(config).to receive(:setup_bundler_env).
with("/my/project/Gemfile")
expect(config).to receive(:setup_bundler_env)
.with("/my/project/Gemfile")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

allow(config).to receive(:exist?).
with(Pathname("/my/project/Gemfile")).and_return(true)
allow(config).to receive(:exist?)
.with(Pathname("/my/project/Gemfile")).and_return(true)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

path = File.expand_path("../../../../bin/guard", __FILE__)
# frozen_string_literal: true

path = File.expand_path("../../../bin/guard", __dir__)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


it "returns nothing if the action response is empty string" do
expect(matched(%w(blank.rb))).to eq [""]
expect(matched(%w[blank.rb])).to eq [""]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


it "returns nothing if the action response is empty string" do
expect(matched(%w(blank.rb))).to eq [""]
expect(matched(%w[blank.rb])).to eq [""]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

klass.new("addition.rb", -> { 1 + 1 }),
klass.new("hash.rb", -> { Hash[:foo, "bar"] }),
klass.new("array.rb", -> { %w(foo bar) }),
klass.new("array.rb", -> { %w[foo bar] }),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


it "returns a single file specified within the action" do
expect(matched(%w(spec_helper.rb))).to eq ["spec"]
expect(matched(%w[spec_helper.rb])).to eq ["spec"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

klass.new("addition.rb", -> { 1 + 1 }),
klass.new("hash.rb", -> { Hash[:foo, "bar"] }),
klass.new("array.rb", -> { %w(foo bar) }),
klass.new("array.rb", -> { %w[foo bar] }),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

let(:pattern) { "foo_spec.rb" }
it "returns the path that matches the string" do
expect(matched(%w(foo_spec.rb foo.rb))).to eq ["foo_spec.rb"]
expect(matched(%w[foo_spec.rb foo.rb])).to eq ["foo_spec.rb"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants