Coder Social home page Coder Social logo

rspec-style-guide's Introduction

RuboCop Logo


Ruby Style Guide Gem Version Actions Status Test Coverage Maintainability Discord

Role models are important.
-- Officer Alex J. Murphy / RoboCop

RuboCop is a Ruby static code analyzer (a.k.a. linter) and code formatter. Out of the box it will enforce many of the guidelines outlined in the community Ruby Style Guide. Apart from reporting the problems discovered in your code, RuboCop can also automatically fix many of them for you.

RuboCop is extremely flexible and most aspects of its behavior can be tweaked via various configuration options.


Patreon OpenCollective OpenCollective Tidelift

Working on RuboCop is often fun, but it also requires a great deal of time and energy.

Please consider financially supporting its ongoing development.

Installation

RuboCop's installation is pretty standard:

$ gem install rubocop

If you'd rather install RuboCop using bundler, add a line for it in your Gemfile (but set the require option to false, as it is a standalone tool):

gem 'rubocop', require: false

RuboCop is stable between minor versions, both in terms of API and cop configuration. We aim to ease the maintenance of RuboCop extensions and the upgrades between RuboCop releases. All big changes are reserved for major releases. To prevent an unwanted RuboCop update you might want to use a conservative version lock in your Gemfile:

gem 'rubocop', '~> 1.65', require: false

See our versioning policy for further details.

Quickstart

Just type rubocop in a Ruby project's folder and watch the magic happen.

$ cd my/cool/ruby/project
$ rubocop

You can also use this magic in your favorite editor with RuboCop's built-in LSP server.

Documentation

You can read a lot more about RuboCop in its official docs.

Compatibility

RuboCop officially supports the following runtime Ruby implementations:

  • MRI 2.7+
  • JRuby 9.4+

Targets Ruby 2.0+ code analysis.

See the compatibility documentation for further details.

Readme Badge

If you use RuboCop in your project, you can include one of these badges in your readme to let people know that your code is written following the community Ruby Style Guide.

Ruby Style Guide

Ruby Style Guide

Here are the Markdown snippets for the two badges:

[![Ruby Style Guide](https://img.shields.io/badge/code_style-rubocop-brightgreen.svg)](https://github.com/rubocop/rubocop)

[![Ruby Style Guide](https://img.shields.io/badge/code_style-community-brightgreen.svg)](https://rubystyle.guide)

Team

Here's a list of RuboCop's core developers:

See the team page for more details.

Logo

RuboCop's logo was created by Dimiter Petrov. You can find the logo in various formats here.

The logo is licensed under a Creative Commons Attribution-NonCommercial 4.0 International License.

Contributors

Here's a list of all the people who have contributed to the development of RuboCop.

I'm extremely grateful to each and every one of them!

If you'd like to contribute to RuboCop, please take the time to go through our short contribution guidelines.

Converting more of the Ruby Style Guide into RuboCop cops is our top priority right now. Writing a new cop is a great way to dive into RuboCop!

Of course, bug reports and suggestions for improvements are always welcome. GitHub pull requests are even better! :-)

Funding

While RuboCop is free software and will always be, the project would benefit immensely from some funding. Raising a monthly budget of a couple of thousand dollars would make it possible to pay people to work on certain complex features, fund other development related stuff (e.g. hardware, conference trips) and so on. Raising a monthly budget of over $5000 would open the possibility of someone working full-time on the project which would speed up the pace of development significantly.

We welcome both individual and corporate sponsors! We also offer a wide array of funding channels to account for your preferences (although currently Open Collective is our preferred funding platform).

If you're working in a company that's making significant use of RuboCop we'd appreciate it if you suggest to your company to become a RuboCop sponsor.

You can support the development of RuboCop via GitHub Sponsors, Patreon, PayPal, Open Collective and Tidelift .

Note: If doing a sponsorship in the form of donation is problematic for your company from an accounting standpoint, we'd recommend the use of Tidelift, where you can get a support-like subscription instead.

Open Collective for Individuals

Support us with a monthly donation and help us continue our activities. [Become a backer]

Open Collective for Organizations

Become a sponsor and get your logo on our README on GitHub with a link to your site. [Become a sponsor]

Changelog

RuboCop's changelog is available here.

Copyright

Copyright (c) 2012-2024 Bozhidar Batsov. See LICENSE.txt for further details.

rspec-style-guide's People

Contributors

3limin4t0r avatar andreareginato avatar andyw8 avatar batshoes avatar bbatsov avatar boardfish avatar brettgoss avatar darhazer avatar dependabot[bot] avatar drewdeponte avatar drujensen avatar edmorley avatar elebow avatar gbpereira avatar goronfreeman avatar hungnh103 avatar justinleveck avatar koic avatar ma2gedev avatar matthew-puku avatar nicholaides avatar noanonoa avatar pcarn avatar pirj avatar russcloak avatar ryanhedges avatar schinery avatar siegfault avatar slabounty avatar ydakuka avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

rspec-style-guide's Issues

Avoid using `before(:context)`

https://www.reddit.com/r/ruby/comments/73nrhk/ama_the_authors_of_effective_testing_with_rspec_3/dnt6vl1/?st=jottx59c&sh=932d4496

in practice it tends to cause problems for users because so many things in RSpec and the surrounding ecosystem assume a per-example lifecycle. For example, if you wrap each example in a DB transaction (as most projects do), and create some DB records in a before(:context) hook, you've just created the records outside of the transaction and those records will leak into other examples and possibly cause them to fail

Restore references to original discussions and sources

After conversion to AsciiDoc (#95) due to significant markup changes, the references to original discussions and sources were lost.
A good place to check them is the result of git blame.

Example:

 Pick "use factories" rule from betterspecs

Original links:
http://www.betterspecs.org/#data
http://lelylan.github.io/betterspecs/#data
Discussion: lelylan/betterspecs#11
https://github.com/lelylan/betterspecs/blob/gh-pages/index.html#L573

Related:
http://blog.steveklabnik.com/posts/2012-07-14-why-i-don-t-like-factory_girl

Since this is a living document, it's very hard to rely on git blame in the long run to keep track of the sources.
Having reference links to discussions in some way would be very helpful for the curious about the reasoning behind the recommendations.

Related to #89

Implicit block expectation syntax

The following syntax is possible

subject { -> { do_something } }
it { is_expected.to change(:something).to(...) }

which is the same as:

it { expect { do_something }.to change(:something).to(...) }

but the former comes with a few drawbacks:

  • it's an exception to the case when subject is not cached
  • it's near impossible to detect with static analysis tools (RuboCop, Reek, ...) that it is actually a block expectation
  • it's rarely used and not known by many

Both examples below will pass:

subject { -> { fail 'Something bad happened' } }
specify do
  is_expected.to raise_error(/Something/)
  is_expected.to raise_error(/bad happened/)
end
before { @a=1 }
subject { -> {@a+=1} }
specify do
  is_expected.to change { @a }.by(1)
  is_expected.to change { @a }.by(1)
  is_expected.to change { @a }.by(1)
  is_expected.to change { @a }.by(1)
  is_expected.to change { @a }.by(1)
  is_expected.to change { @a }.by(1)
end

Keeping in mind that:

  • we don't recommend using an implicit subject (should/is_expected) except in some cases,
  • the benefit of using this syntax is quite low

There seem to be not much usage cases left for implicit block expectation syntax.
What do you think of recommending against this syntax?

Related blog post on blog.rubystyle.guide, reddit comments.

Convert to AsciiDoc

In light of #87 I think it'd be nice if we adopted a better format for the guide, namely AsciiDoc. It would allow us to have native admonitions, footnotes, sane nesting of list items, etc and to easily export the guide in various formats. That would also make it trivial to publish a simple site from the guide down the road if we want to.

It's trivial to convert to AsciiDoc with this tool https://github.com/asciidoctor/kramdown-asciidoc (created by the maintainer of AsciiDoctor himself).

In general my longer term plan is to convert all guides to AsciiDoc and maybe even RuboCop's own documentation.

Recommend using `shared_context`

  1. shared_context is undeservedly omitted from the style guide, and it's sometimes extremely useful.

  2. It makes sense to add another rule at the same time to describe the difference between shared_example and shared_context, so they are not used interchangeably, the latter should not contain examples/groups.

Remove mentions of tools?

I think mentioning workflow tools such as a guard and fuubar is out of scope for a style guide and should be removed.

Relax factories vs fixtures rule

Fixtures as something created before suite might be useful, specifically e.g. for a list of countries that are later used by other records (via say, user.country_id especially when there's a foreign key constraint).
There should be a balance between creating all the records, as this may negatively impact the performance when only a few examples are run vs save big when multiple examples are run.

Unfortunately, in Rails, the Rails fixtures are being loaded before each example, and that neglects the benefits, even despite the recently introduced bulk loading.

Discourage stubbing subject

Some times people stub helper methods in the object they are testing, but that's a test smell and often indicate a bad design on the object itself.

RSpec/SubjectStub cop flags allow and expect on the subject

Recommend using `pry-resque/rspec`

It's very useful to stop at breakpoint where the test is about to break with possibilities to make changes to code/context and re-run the examples on the fly, without leaving the console.

Names subjects

Rubocop-RSpec forces the usage of named subject, when the subject is referenced as in the example below:

describe Article do
  subject { FactoryGirl.create(:article) }

  it 'is not published on creation' do
    expect(subject).not_to be_published
  end
end

And in fact it becomes even more readable:

describe Article do
  subject(:article) { FactoryGirl.create(:article) }

  it 'is not published on creation' do
    expect(article).not_to be_published
  end
end

So we might want to reword the "Use subject when possible" rule

Mention the built-in alternative to DatabaseCleaner for Rails

DatabaseCleaner was never meant to be used in Rails apps, and while still allowing the truncation and deletion strategies that some may find useful (I can't imagine in which cases though).

One of my motivations for writing this library was to have an easy way to turn on what Rails calls "transactional_fixtures" in my non-rails ActiveRecord projects.

Actually transactional_fixtures have nothing to do with fixtures, and have been recently renamed to transactional_tests to reflect what they are intended to be used for, wrap every example in a transaction and roll it back after example.

Originally found in https://anti-pattern.com/transactional-fixtures-in-rails

Recommend preferring helper methods to hooks

Don't overuse hooks, prefer explicitness.
Helper methods are more explicit and even provide more flexibility

An example off the top of my head:

# bad
let(:article) { Article.new }

before do
  article.author = author
end

context 'with an author' do
  let(:author) { Author.new('John') }

  it "returns article's author name" do
    expect(article.author_name).to eq 'John'
  end
end

context 'without an author' do
  let(:author) { nil }

  it "returns a placeholder" do
    expect(article.author_name).to eq 'Unknown'
  end
end

# good
def article_for_author(author)
  article = Article.new
  article.author = author
  article
end

context 'with an author' do
  let(:author) { Author.new('John') }

  it "returns article's author name" do
    article = article_for_author(author)
    expect(article.author_name).to eq 'John'
  end
end

context 'without an author' do
  it "returns a placeholder" do
    article = article_for_author(nil)
    expect(article.author_name).to eq 'Unknown'
  end
end

Just like with #56, this should be used when there's a compelling benefit for extraction - improved readability or code reuse.

Advise using `verify_doubled_constant_names`

In addition to verify_partial_doubles the above option may come handy to verify that you are stubbing an existing constant.

      # When this is set to true, an error will be raised when
      # `instance_double` or `class_double` is given the name of an undefined
      # constant. You probably only want to set this when running your entire
      # test suite, with all production code loaded. Setting this for an
      # isolated unit test will prevent you from being able to isolate it!

Example

With this option set to false the following example will, surprisingly, pass:

describe UserGreeter do
  it 'picks user name and prints greeting' do
    user = instance_double('Usor') # notice a typo
    allow(user).to receive(:name).and_return('John')
    expect { subject.process(user) }.to output('Hello, John')
end

When User renames its name method to full_name, this spec will still pass, but UserGreeter will blow up in production with NoMethodError.

Real-life Examples

Example violations (resulting with errors when the option is set to true):

# Typo: notice the leading space
let(:api) { instance_double(' ThirdParty::API') }

# `instance_double` expects a class name
let(:charge) { instance_double('payment', id: 1) } # "payment" is not a defined constant. Perhaps you misspelt it?

# Does not exist outside of the test code
let(:action) { instance_double('Action') } # "Action" is not a defined constant (though "Namespace::Action" is)

# Not loaded
let(:settings) { class_double('Settings') } # "Settings" is not a defined constant

# Double stubbing
before do
  stub_const('ActionClass', Class.new)
end
let(:action) { instance_double(ActionClass) } "ActionClass" is not a defined constant

Confusing `described_class`

Hey, everyone!

described_class is no doubt widely used in specs, there are 1376 usages in repositories listed in real-world-ruby-apps.

According to described_class docs:

If the first argument to [...] example group is a class, the class is exposed to each example via the described_class() method.

However, in fact it's slightly different:

It's more like described_thing -- which is a class if you pass a class or whatever object you pass.

-- Myron Marston

With one exception though, when a string is passed as the first argument.

The reason for the string exception is that strings, when passed to describe or context, are taken to be an English description of the context, and not the object-being-described.
Any other type of object is assumed to be the thing the user is describing.

-- Myron Marston

One common mistake is passing metadata without a proper docstring, e.g.:

RSpec.describe SomeClass do
  describe controller: true do
    it { expect(described_class).to eq(SomeClass) }
  end
end

results in:

  1) SomeClass {:controller=>true} should eq SomeClass
     Failure/Error: it { expect(described_class).to eq(SomeClass) }

       expected: SomeClass
            got: {:controller=>true}

It may be unclear why the first argument is taken from the innermost example group, not outermost (SomeClass).
There was a breaking change introduced in RSpec 3.0 to take the first argument of the innermost example group as described_class, not outermost.
Unfortunately, this was not reflected in the docs (1, 2), and it's still not (it's on master but not on 3-8-maintenance), even after this pull request.

I also argue against using classes there (with describe) preferring strings

-- Jon Rowe

# bad
RSpec.describe Article do

# good
RSpec.describe “Article” do

I recommend sticking to describe <string arg> for nested example groups as it avoids confusion.

-- Myron Marston

I'd naively expect described_class to return "the class of the described thing" (i.e. Class when describing an actual class). Leaving open because this is definitely still an inconsistency.

-- Xavier Shay

There was an idea to change described_class's behaviour:

describe [1,2,3] do
  it { described_class.should eq(Array) }
  it { subject.should be_a(Array) }
end

Unfortunately, this idea was two weeks late for RSpec 3.0 release.

I like your idea, @betesh. I wish we had thought of it before 3.0 shipped.

-- Myron Marston

With all this confusion in mind:

I think in RSpec 4 we should just remove described_class, we kind of consider it an anti-pattern (because using it means the class must be loaded when the file is evaluated rather than when it's run) and we thus don't recommend it, and it causes this kind of ambiguity.

-- Jon Rowe

Bottom Line

Pros of described_class:

  • is widely used
  • reads nicely (when used properly)
  • reduces churn if the name of the class under test changes

Cons of described_class:

  • leads to confusion with nested example groups
  • leads to confusion when metadata is used without docstring
  • leads to confusion when something that is neither a String nor a Class is passed to example group as the first argument
  • might disappear in RSpec 4.0

How to Deal with This

What do you think about introducing a guideline to discourage passing anything than a literal class and a string as the first argument to an example group?
React with a 🚀 if you agree.

Or would you be radical and discourage the use of described_class altogether, and recommend using class names explicitly?
Place a 👍 if you decide to be explicit about what is being tested.

Please react with 👎 if you'd like to stick to described_class (at least until RSpec 4 is out).

Highly appreciate if you add additional arguments pro and contra.

"context descriptions" when there are independent clauses

context block descriptions should always start with 'when', and be in the form of a sentence with proper grammar.

What if we have independent clauses. Then would it be permissible to join with an and when?

e.g.:

context 'when condition A is true' do
  context 'and when condition B is false' do
    ...
  end
  context 'and when condition B is true' do
    ...
  end
end

Then the output from --format documentation reads like a proper sentence. Otherwise, if you force each block to start with "when", it doesn't make readable sentences.

Recommend using `it_behaves_like` vs `include_examples`

The difference between the two is that the former creates a context, while the latter includes the examples and groups together with the context defined in the shared_examples that may lead to undesired consequences as overridden declarations.
I don't have good use cases for the latter in mind.

Mention Crystallball

CrystallBall is the library implementing Regression Test Selection, helping you to only run the tests that are affected by the code changed. Might be a good fit for projects with large spec suites.

We can not use expect in a describe block, can we?

Noticed here as a good example:

# Refactored
describe '#attributes' do
  subject { FactoryGirl.create(:article) }

  expect(subject.attributes).to include subject.display_name
  expect(subject.attributes).to include subject.created_at
end

Clean up the stale branches

Some of the branches contain meaningful commits. Consider picking them.
Drop the unused branches afterward.

Don't Recommend let or let!

Opening this per @bbatsov's response to my comment on Reddit.

The meat of my argument (as copied from here):

There's a comment from Myron Marston, long time RSpec maintainer explaining this...

I've seen Marston's arguments from 2011 and mostly have never agreed. Here ~7 years later in 2018, it seems he may be starting to realize they're bad:

My rule of thumb is to use them when there's a compelling benefit,...
... but don't reach for them as the default way to write specs.

No. There's never a compelling benefit: interpreter detecting your typo and lazy loading objects under test are not compelling.

If I was concerned about an interpreter detecting a typo I'd be using a different language.

Lazy loading, what is the point? If your tests are consuming resources to the point that you need to lazy evaluate them your tests have a problem.

Remember, we're talking maintenance phase here: people need to go back to tests written months and years ago. Tracing through lazy loaded let hierarchies sprinkled with eager loaded let! with before hooks and shared examples that depend on the caller to define more lets is not maintainable.

Obscure tests is not a compelling benefit.

And really, we're talking maintainability and style, so consistency is important. Let's throwout the very important argument above and consider this. It's safe to say (I think) that after 20+ years the TMTOWTDI camp has lost out to the There should be one—and preferably only one—obvious way to do it camp. I mean that's why we're here talking style guides, right? We don't want TMTOWTDI.

In this case not everything can be done with let, you need let!. And not everything can be done with let, you need before. Why have some tests use X and some use Y and some use X and Y and maybe Z? Be consistent. KISS: use before. It's all you need.

Relax context wording rule (allow with and without)

Currently the style guide says that context should always start with when, but with and without are also good descriptions of context and they are allowed by default in rubocop-rspec.

So I suggest to add them to the style guide as well.

rubocop-rspec pull request
betterspecs pull request

Rationale:

# not so hot
context 'with sprinkles' ...
context 'when there are no sprinkles on top' ...

# better
context 'with sprinkles' ...
context 'without sprinkles' ...

How to cite sources?

For example:

Use :context instead of the ambiguous :all scope in before/after hooks.

There's a section in Effective Testing With RSpec 3 which provides similar advice:

https://books.google.ca/books?id=8g5QDwAAQBAJ&pg=PT195&lpg=PT195

I think it would be useful to cite this as an 'authoritative' reference (since the book is written by an RSpec maintainer), but I'm not sure what's the most appropriate way. Should it link to Google Books? Or Safari Books Online (which requires paid access)? Or should it just mention the book by its title?

Encourage to use local matchers

E.g.

  matcher :be_just_like do |expected|
    match {|actual| actual == expected}
  end

Related docs:
https://relishapp.com/rspec/rspec-expectations/v/3-8/docs/custom-matchers/define-a-custom-matcher#scoped-in-a-module
https://relishapp.com/rspec/rspec-expectations/v/3-8/docs/custom-matchers/define-a-custom-matcher#scoped-in-an-example-group

Caveats

I personally find it very confusing that those matchers don't have access to group variables, as opposed to ad-hoc methods:

def send_message
  have_attributes(channel: channel, text: text)
end

let(:channel) { '#announcements' }
let(:text) { 'Hello, world' }

it { is_expected.to send_message }

With a matcher:

let(:channel) { '#announcements' }
let(:text) { 'Hello, world' }

# works
matcher :send_message do |channel, text|
  values_match? actual, have_attributes(channel: channel, text: text)
end

it { is_expected.to send_message(channel, text) }

# fails with
#     undefined local variable or method `channel' for #<Class:#<RSpec::Matchers::DSL::Matcher:0x00007fb334216058>> 

matcher :send_message do
  values_match? actual, have_attributes(channel: channel, text: text)
end

it { is_expected.to send_message }

Explain how metadata work

Metadata is very useful and is able to include shared contexts, include group/example-specific helpers and hooks.

Difference between subject and let

I've spent the last half hour trying to google the difference between subject and let, after having already read the RSpec docs, which is how I came across this guide. Which I'm thankfully for, because there's lots of useful suggestions here.

However, when I finally got to the subject/lets section, my excitement turned to disappointment. I still don't understand the difference between subject and let...

Help?

Use `before` and `let` when it's beneficial

Only use before and let, when there's a compelling benefit, don't use them as the default way to write specs.
https://reddit.com/r/ruby/comments/73nrhk/ama_the_authors_of_effective_testing_with_rspec_3/dnsyanp/

An example I can think of:

# bad
describe '#filename' do
  let(:filename) { 'input.csv' }

  before { settings.load!(filename) }

  specify do
    expect(settings.filename).to eq('input.csv')
  end
end

# good
describe '#filename' do
  it 'returns filename' do
    filename = 'input.csv'
    settings.load!(filename)

    expect(settings.filename).to eq('input.csv')
  end
end

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.