From ae6e711768e5e557019b021a996b9a0c070a1d58 Mon Sep 17 00:00:00 2001 From: "Andrew Kozin (aka nepalez)" Date: Sun, 12 Feb 2017 14:10:39 +0300 Subject: [PATCH] Add coverage --- README.md | 117 +++++++++++++++--- lib/rspec/sqlimit.rb | 46 ++++--- lib/rspec/sqlimit/counter.rb | 28 +++-- lib/rspec/sqlimit/reporter.rb | 30 +++-- spec/dummy/app/models/post.rb | 3 - .../db/migrate/20170211104800_create_users.rb | 4 +- .../db/migrate/20170211104823_create_posts.rb | 9 -- spec/dummy/db/schema.rb | 10 +- spec/dummy/db/test.sqlite3 | Bin 10240 -> 7168 bytes spec/dummy/lib/dummy.rb | 1 - spec/rspec/sqlimit_spec.rb | 30 ++++- 11 files changed, 188 insertions(+), 90 deletions(-) delete mode 100644 spec/dummy/app/models/post.rb delete mode 100644 spec/dummy/db/migrate/20170211104823_create_posts.rb diff --git a/README.md b/README.md index 3b7df81..90b62a8 100644 --- a/README.md +++ b/README.md @@ -1,41 +1,126 @@ -# Rspec::Sqlimit +# Test-Driven way of fighting N + 1 queries -Welcome to your new gem! In this directory, you'll find the files you need to be able to package up your Ruby library into a gem. Put your Ruby code in the file `lib/rspec/sqlimit`. To experiment with that code, run `bin/console` for an interactive prompt. +RSpec matcher to control number of SQL queries executed by a block of code. -TODO: Delete this and the text above, and describe your gem +It wraps [the answer at Stack Overflow][stack-answer] by [Ryan Bigg][ryan-bigg], which based on Active Support [Notification][notification] and [Instrumentation][instrumentation] mechanisms. + +[![Gem Version][gem-badger]][gem] +[![Build Status][travis-badger]][travis] +[![Dependency Status][gemnasium-badger]][gemnasium] +[![Code Climate][codeclimate-badger]][codeclimate] + + +Sponsored by Evil Martians ## Installation -Add this line to your application's Gemfile: +```ruby +# Gemfile +gem "rspec-sqlimit" +``` + +## Usage + +The gem defines matcher `exceed_query_limit` that takes maximum number of SQL requests to be made inside the block. ```ruby -gem 'rspec-sqlimit' +require "rspec-sqlimit" + +RSpec.describe "N+1 safety" do + it "doesn't send unnecessary requests to db" do + expect { User.create }.not_to exceed_query_limit(1) + end +end ``` -And then execute: +The above specification fails with the following description: - $ bundle +``` +Failure/Error: expect { User.create }.not_to exceed_query_limit(1) -Or install it yourself as: + Expected to run maximum 1 queries + The following 3 queries were invoked: + 1) begin transaction (0.045 ms) + 2) INSERT INTO "users" DEFAULT VALUES (0.19 ms) + 3) commit transaction (148.935 ms) +``` - $ gem install rspec-sqlimit +You can restrict the matcher using regex: -## Usage +```ruby +require "rspec-sqlimit" + +RSpec.describe "N+1 safety" do + it "doesn't send unnecessary requests to db" do + expect { User.create }.not_to exceed_query_limit(1).with(/^INSERT/) + end +end +``` -TODO: Write usage instructions here +This time test passes. -## Development +When a specification with a restriction fails, you'll see an error as follows: -After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake spec` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. +```ruby +require "rspec-sqlimit" + +RSpec.describe "N+1 safety" do + it "doesn't send unnecessary requests to db" do + expect { User.create }.not_to exceed_query_limit(1).with(/^INSERT/) + end +end +``` + +``` +Failure/Error: expect { User.create }.not_to exceed_query_limit(0).with(/INSERT/) + + Expected to run maximum 0 queries that match (?-mix:INSERT) + The following 1 queries were invoked among others (see mark ->): + 1) begin transaction (0.072 ms) + -> 2) INSERT INTO "users" DEFAULT VALUES (0.368 ms) + 3) commit transaction (147.559 ms) +``` -To install this gem onto your local machine, run `bundle exec rake install`. To release a new version, update the version number in `version.rb`, and then run `bundle exec rake release`, which will create a git tag for the version, push git commits and tags, and push the `.gem` file to [rubygems.org](https://rubygems.org). +## Further Development -## Contributing +For now the gem uses unbinded Active Record queries in error descriptions. For example, when your query contains arguments, the error message will look like -Bug reports and pull requests are welcome on GitHub at https://github.com/[USERNAME]/rspec-sqlimit. +```ruby +require "rspec-sqlimit" + +RSpec.describe "N+1 safety" do + it "doesn't send unnecessary requests to db" do + expect { User.create(name: "Joe") }.not_to exceed_query_limit(1) + end +end +``` + +``` +Failure/Error: expect { User.create }.not_to exceed_query_limit(0).with(/INSERT/) + + Expected to run maximum 0 queries that match (?-mix:INSERT) + The following 1 queries were invoked among others (see mark ->): + 1) begin transaction (0.072 ms) + -> 2) INSERT INTO "users" ("name") VALUES (?) (0.368 ms) + 3) commit transaction (147.559 ms) +``` +This is because [Active Record instrumentation hook][hook] keeps a query and bindings separately (under `:sql` and `:binds` keys). So the challenge is **to bind arguments to the query** in the report to make a debugging a bit simpler. ## License The gem is available as open source under the terms of the [MIT License](http://opensource.org/licenses/MIT). +[codeclimate-badger]: https://img.shields.io/codeclimate/github/nepalez/rspec-sqlimit.svg?style=flat +[codeclimate]: https://codeclimate.com/github/nepalez/rspec-sqlimit +[gem-badger]: https://img.shields.io/gem/v/rspec-sqlimit.svg?style=flat +[gem]: https://rubygems.org/gems/rspec-sqlimit +[gemnasium-badger]: https://img.shields.io/gemnasium/nepalez/rspec-sqlimit.svg?style=flat +[gemnasium]: https://gemnasium.com/nepalez/rspec-sqlimit +[travis-badger]: https://img.shields.io/travis/nepalez/rspec-sqlimit/master.svg?style=flat +[travis]: https://travis-ci.org/nepalez/rspec-sqlimit +[stack-answer]: http://stackoverflow.com/a/5492207/1869912 +[ryan-bigg]: http://ryanbigg.com/ +[notification]: http://api.rubyonrails.org/classes/ActiveSupport/Notifications.html +[instrumentation]: http://guides.rubyonrails.org/active_support_instrumentation.html +[hook]: http://guides.rubyonrails.org/active_support_instrumentation.html#sql-active-record diff --git a/lib/rspec/sqlimit.rb b/lib/rspec/sqlimit.rb index d5921f6..7c36a3a 100644 --- a/lib/rspec/sqlimit.rb +++ b/lib/rspec/sqlimit.rb @@ -1,3 +1,4 @@ +require "active_support/notifications" require "active_record" require "rspec" @@ -7,45 +8,40 @@ module SQLimit require_relative "sqlimit/reporter" end - Matchers.define :exceed_query_limit do |expected, matcher = nil| - def counter - @counter ||= RSpec::SQLimit::Counter.new - end - - def message(expected, matcher, negation = false) - if matcher - condition = negation ? "maximum" : "more than" - restriction = " that match #{matcher}" - suffix = " among others (see mark ->)" - end - - reporter = RSpec::SQLimit::Reporter.new(counter, matcher) - - <<-MESSAGE.gsub(/ +\|/, "") - |Expected to run #{condition} #{expected} queries#{restriction} - |The following #{reporter.count} queries were invoked#{suffix}: - |#{reporter.lines.join("\n")} - MESSAGE + Matchers.define :exceed_query_limit do |expected| + chain :with do |matcher| + @matcher = matcher end match do |block| - counter.count(&block) - RSpec::SQLimit::Reporter.new(counter, matcher).count > expected + @counter ||= RSpec::SQLimit::Counter[@matcher, block] + @counter.count > expected end match_when_negated do |block| - counter.count(&block) - RSpec::SQLimit::Reporter.new(counter, matcher).count <= expected + @counter ||= RSpec::SQLimit::Counter[@matcher, block] + @counter.count <= expected end failure_message do |_| - message(expected, matcher) + message(expected, @counter) end failure_message_when_negated do |_| - message(expected, matcher, true) + message(expected, @counter, true) end supports_block_expectations + + def message(expected, counter, negation = false) + reporter = RSpec::SQLimit::Reporter.new(counter) + condition = negation ? "maximum" : "more than" + restriction = " that match #{reporter.matcher}" if reporter.matcher + + <<-MESSAGE.gsub(/ +\|/, "") + |Expected to run #{condition} #{expected} queries#{restriction} + |#{reporter.call} + MESSAGE + end end end diff --git a/lib/rspec/sqlimit/counter.rb b/lib/rspec/sqlimit/counter.rb index a38afeb..17d214e 100644 --- a/lib/rspec/sqlimit/counter.rb +++ b/lib/rspec/sqlimit/counter.rb @@ -1,23 +1,35 @@ module RSpec::SQLimit class Counter - attr_reader :queries + attr_reader :queries, :matcher - def initialize - @queries = [] + def self.[](*args) + new(*args).tap(&:call) end - # FIXME add mutex to ensure thread-safety - def count + def initialize(matcher, block) @queries = [] - ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do - yield + @matcher = matcher + @block = block + @mutex = Mutex.new + end + + def call + @mutex.synchronize do + @queries = [] + ActiveSupport::Notifications.subscribed callback, "sql.active_record" do + @block.call + end end end + def count + matcher ? queries.count { |query| query[:sql] =~ matcher } : queries.count + end + private def callback - @callback ||= lambda(_name, start, finish, _message_id, values) do + @callback ||= lambda do |_name, start, finish, _message_id, values| return if %w(CACHE SCHEMA).include? values[:name] queries << { sql: values[:sql], duration: (finish - start) * 1_000 } end diff --git a/lib/rspec/sqlimit/reporter.rb b/lib/rspec/sqlimit/reporter.rb index c8b28bd..ab3ddaf 100644 --- a/lib/rspec/sqlimit/reporter.rb +++ b/lib/rspec/sqlimit/reporter.rb @@ -2,29 +2,33 @@ module RSpec::SQLimit class Reporter attr_reader :matcher - def initialize(counter, matcher = nil) + def initialize(counter) @counter = counter - @matcher = matcher + @count = counter.count + @queries = counter.queries + @matcher = counter.matcher end - def count - return queries.count unless @matcher - queries.count { |query| query[:sql] =~ @matcher } - end + def call + suffix = " among others (see mark ->)" if @matcher - def lines - queries.map.with_index { |*args| line(*args) } + return "No queries were invoked" if @queries.empty? + + <<-MESSAGE.gsub(/ +\|/, "") + |The following #{@count} queries were invoked#{suffix}: + |#{lines.join("\n")} + MESSAGE end private - def line(query, index) - prefix = @matcher && query =~ @matcher ? "->" : " " - "#{prefix} #{index}) #{query[:sql]} (#{query[:duration].round(3)} ms)" + def lines + @queries.map.with_index { |*args| line(*args) } end - def queries - @queries ||= @counter.queries + def line(query, index) + prefix = (matcher && query[:sql] =~ matcher) ? "->" : " " + "#{prefix} #{index + 1}) #{query[:sql]} (#{query[:duration].round(3)} ms)" end end end diff --git a/spec/dummy/app/models/post.rb b/spec/dummy/app/models/post.rb deleted file mode 100644 index 542c073..0000000 --- a/spec/dummy/app/models/post.rb +++ /dev/null @@ -1,3 +0,0 @@ -class Post < ActiveRecord::Base - belongs_to :user -end diff --git a/spec/dummy/db/migrate/20170211104800_create_users.rb b/spec/dummy/db/migrate/20170211104800_create_users.rb index 20f0c89..da924da 100644 --- a/spec/dummy/db/migrate/20170211104800_create_users.rb +++ b/spec/dummy/db/migrate/20170211104800_create_users.rb @@ -1,7 +1,5 @@ class CreateUsers < ActiveRecord::Migration def change - create_table :users do |t| - t.string :name, index: true - end + create_table :users end end diff --git a/spec/dummy/db/migrate/20170211104823_create_posts.rb b/spec/dummy/db/migrate/20170211104823_create_posts.rb deleted file mode 100644 index 6567ae4..0000000 --- a/spec/dummy/db/migrate/20170211104823_create_posts.rb +++ /dev/null @@ -1,9 +0,0 @@ -class CreatePosts < ActiveRecord::Migration - def change - create_table :posts do |t| - t.references :users - t.string :title - t.text :text - end - end -end diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index d7a23e0..c10f262 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -10,17 +10,9 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170211104823) do - - create_table "posts", force: :cascade do |t| - t.integer "users_id" - t.string "title" - t.text "text" - end +ActiveRecord::Schema.define(version: 20170211104800) do create_table "users", force: :cascade do |t| - t.string "name" - t.index ["name"], name: "index_users_on_name" end end diff --git a/spec/dummy/db/test.sqlite3 b/spec/dummy/db/test.sqlite3 index 01617835f18c59b58bbfec691f3e64b55e02aa3f..b99ac21a4fad52e43db466e7201be97e55a47f37 100644 GIT binary patch delta 145 zcmZn&Xt0k_Zb26Z0Jg=6B3@ zfC3kog#?(GG>r@l%?*qU4Gj%UER2jNzhf5zD!L6)bQ`GXJhQM6vnC^|BBssnxTSbD zbMXA(7U5v1VgRBqjOI)nOo2=Xn1z`0CN?HbwrAeES(QnOku#T-U0ht8vC(q!dB#vd z9R;Pl#N1RRg|ft=