From a27e5939240e13c5fb45a73fc9ad1717d297b06e Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 7 Jan 2016 10:51:39 +1100 Subject: [PATCH 01/13] Fix deprecation notices --- spec/controllers/spree/orders_controller_spec.rb | 6 +++--- spec/spec_helper.rb | 2 +- spec/support/request/web_helper.rb | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index d95012a6fb..73c72f3386 100644 --- a/spec/controllers/spree/orders_controller_spec.rb +++ b/spec/controllers/spree/orders_controller_spec.rb @@ -58,21 +58,21 @@ describe Spree::OrdersController do end it "returns HTTP success when successful" do - Spree::OrderPopulator.stub(:new).and_return(populator = mock()) + Spree::OrderPopulator.stub(:new).and_return(populator = double()) populator.stub(:populate).and_return true xhr :post, :populate, use_route: :spree, format: :json response.status.should == 200 end it "returns failure when unsuccessful" do - Spree::OrderPopulator.stub(:new).and_return(populator = mock()) + Spree::OrderPopulator.stub(:new).and_return(populator = double()) populator.stub(:populate).and_return false xhr :post, :populate, use_route: :spree, format: :json response.status.should == 402 end it "tells populator to overwrite" do - Spree::OrderPopulator.stub(:new).and_return(populator = mock()) + Spree::OrderPopulator.stub(:new).and_return(populator = double()) populator.should_receive(:populate).with({}, true) xhr :post, :populate, use_route: :spree, format: :json end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d9a1e1b262..61552c528d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -39,7 +39,7 @@ Capybara.register_driver :poltergeist do |app| Capybara::Poltergeist::Driver.new(app, options) end -Capybara.default_wait_time = 30 +Capybara.default_max_wait_time = 30 require "paperclip/matchers" diff --git a/spec/support/request/web_helper.rb b/spec/support/request/web_helper.rb index 15c586561c..1dd5adc7a3 100644 --- a/spec/support/request/web_helper.rb +++ b/spec/support/request/web_helper.rb @@ -119,7 +119,7 @@ module WebHelper # Do not use this without good reason. Capybara's built-in waiting is very effective. def wait_until(secs=nil) require "timeout" - Timeout.timeout(secs || Capybara.default_wait_time) do + Timeout.timeout(secs || Capybara.default_max_wait_time) do sleep(0.1) until value = yield value end From e24027a8d0ebf8fa3bd43a4e1b832ef43b43e173 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 7 Jan 2016 10:52:36 +1100 Subject: [PATCH 02/13] Speed up add to cart: Update the order once per fee calculation, rather than for every line item x fee --- app/models/spree/adjustment_decorator.rb | 14 ++++++++++++ app/models/spree/order_decorator.rb | 28 +++++++++++++++--------- spec/models/spree/order_spec.rb | 6 +++++ 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/app/models/spree/adjustment_decorator.rb b/app/models/spree/adjustment_decorator.rb index fb3b190394..0d1bc941f6 100644 --- a/app/models/spree/adjustment_decorator.rb +++ b/app/models/spree/adjustment_decorator.rb @@ -35,5 +35,19 @@ module Spree def display_included_tax Spree::Money.new(included_tax, { :currency => currency }) end + + def self.without_callbacks + skip_callback :save, :after, :update_adjustable + skip_callback :destroy, :after, :update_adjustable + + result = yield + + ensure + set_callback :save, :after, :update_adjustable + set_callback :destroy, :after, :update_adjustable + + result + end + end end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 8f1c0381b1..53e80bbdef 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -164,21 +164,29 @@ Spree::Order.class_eval do def update_distribution_charge! with_lock do - EnterpriseFee.clear_all_adjustments_on_order self - line_items.each do |line_item| - if provided_by_order_cycle? line_item - OpenFoodNetwork::EnterpriseFeeCalculator.new.create_line_item_adjustments_for line_item + # Without intervention, the Spree::Adjustment#update_adjustable callback is called + # once for every (line item x fee), which triggers a costly Spree::Order#update! + Spree::Adjustment.without_callbacks do + EnterpriseFee.clear_all_adjustments_on_order self - else - pd = product_distribution_for line_item - pd.create_adjustment_for line_item if pd + line_items.each do |line_item| + if provided_by_order_cycle? line_item + OpenFoodNetwork::EnterpriseFeeCalculator.new.create_line_item_adjustments_for line_item + + else + pd = product_distribution_for line_item + pd.create_adjustment_for line_item if pd + end + end + + if order_cycle + OpenFoodNetwork::EnterpriseFeeCalculator.new.create_order_adjustments_for self end end - if order_cycle - OpenFoodNetwork::EnterpriseFeeCalculator.new.create_order_adjustments_for self - end + # After fees are calculated, we update the order once + update! end end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index e50a64a4d9..300cf26bac 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -54,6 +54,8 @@ describe Spree::Order do product_distribution.should_receive(:create_adjustment_for).with(line_item) subject.stub(:product_distribution_for) { product_distribution } + subject.should_receive(:update!) + subject.update_distribution_charge! end @@ -65,6 +67,8 @@ describe Spree::Order do subject.stub(:product_distribution_for) { nil } + subject.should_receive(:update!) + subject.update_distribution_charge! end @@ -90,6 +94,8 @@ describe Spree::Order do OpenFoodNetwork::EnterpriseFeeCalculator.any_instance.stub(:create_order_adjustments_for) subject.stub(:order_cycle) { order_cycle } + subject.should_receive(:update!) + subject.update_distribution_charge! end From ac650ebd46ac0e161865ec9821124a179e18a65e Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 7 Jan 2016 10:56:56 +1100 Subject: [PATCH 03/13] Add support for mirroring Norway's database --- script/mirror_db.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/script/mirror_db.sh b/script/mirror_db.sh index 85e6693b68..4c6142b701 100755 --- a/script/mirror_db.sh +++ b/script/mirror_db.sh @@ -10,12 +10,20 @@ else RAILS_RUN='bundle exec rails runner' fi +if [[ $1 != 'ofn-no' ]]; then + DB_USER='openfoodweb' + DB_DATABASE='openfoodweb_production' +else + DB_USER='ofn_user' + DB_DATABASE='openfoodnetwork' +fi + # -- Mirror database echo "Mirroring database..." echo "drop database open_food_network_dev" | psql -h localhost -U ofn open_food_network_test echo "create database open_food_network_dev" | psql -h localhost -U ofn open_food_network_test -ssh $1 "pg_dump -h localhost -U openfoodweb openfoodweb_production |gzip" |gunzip |psql -h localhost -U ofn open_food_network_dev +ssh $1 "pg_dump -h localhost -U $DB_USER $DB_DATABASE |gzip" |gunzip |psql -h localhost -U ofn open_food_network_dev # -- Disable S3 From 257b5a9eefa964be7b9fc48b28bee743c409e50e Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 7 Jan 2016 11:21:25 +1100 Subject: [PATCH 04/13] Move premature Spree::Order#update prevention up a level, for even greater efficiency gains --- .../spree/orders_controller_decorator.rb | 24 +++++++++++----- app/models/spree/order_decorator.rb | 28 +++++++------------ spec/models/spree/order_spec.rb | 5 ---- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index fd5d3b634e..c6d325d0f8 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -23,13 +23,23 @@ Spree::OrdersController.class_eval do end def populate - populator = Spree::OrderPopulator.new(current_order(true), current_currency) - if populator.populate(params.slice(:products, :variants, :quantity), true) - fire_event('spree.cart.add') - fire_event('spree.order.contents_changed') - render json: true, status: 200 - else - render json: false, status: 402 + # Without intervention, the Spree::Adjustment#update_adjustable callback is called many times + # during cart population, for both taxation and enterprise fees. This operation triggers a + # costly Spree::Order#update!, which only needs to be run once. We avoid this by disabling + # callbacks on Spree::Adjustment and then manually invoke Spree::Order#update! on success. + + Spree::Adjustment.without_callbacks do + populator = Spree::OrderPopulator.new(current_order(true), current_currency) + if populator.populate(params.slice(:products, :variants, :quantity), true) + fire_event('spree.cart.add') + fire_event('spree.order.contents_changed') + + current_order.update! + + render json: true, status: 200 + else + render json: false, status: 402 + end end end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 53e80bbdef..8f1c0381b1 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -164,29 +164,21 @@ Spree::Order.class_eval do def update_distribution_charge! with_lock do + EnterpriseFee.clear_all_adjustments_on_order self - # Without intervention, the Spree::Adjustment#update_adjustable callback is called - # once for every (line item x fee), which triggers a costly Spree::Order#update! - Spree::Adjustment.without_callbacks do - EnterpriseFee.clear_all_adjustments_on_order self + line_items.each do |line_item| + if provided_by_order_cycle? line_item + OpenFoodNetwork::EnterpriseFeeCalculator.new.create_line_item_adjustments_for line_item - line_items.each do |line_item| - if provided_by_order_cycle? line_item - OpenFoodNetwork::EnterpriseFeeCalculator.new.create_line_item_adjustments_for line_item - - else - pd = product_distribution_for line_item - pd.create_adjustment_for line_item if pd - end - end - - if order_cycle - OpenFoodNetwork::EnterpriseFeeCalculator.new.create_order_adjustments_for self + else + pd = product_distribution_for line_item + pd.create_adjustment_for line_item if pd end end - # After fees are calculated, we update the order once - update! + if order_cycle + OpenFoodNetwork::EnterpriseFeeCalculator.new.create_order_adjustments_for self + end end end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 300cf26bac..4d681e01cc 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -54,7 +54,6 @@ describe Spree::Order do product_distribution.should_receive(:create_adjustment_for).with(line_item) subject.stub(:product_distribution_for) { product_distribution } - subject.should_receive(:update!) subject.update_distribution_charge! end @@ -67,8 +66,6 @@ describe Spree::Order do subject.stub(:product_distribution_for) { nil } - subject.should_receive(:update!) - subject.update_distribution_charge! end @@ -94,8 +91,6 @@ describe Spree::Order do OpenFoodNetwork::EnterpriseFeeCalculator.any_instance.stub(:create_order_adjustments_for) subject.stub(:order_cycle) { order_cycle } - subject.should_receive(:update!) - subject.update_distribution_charge! end From 71569324f4b6c1549cec6127e7122f307397dd1d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 7 Jan 2016 13:53:37 +1100 Subject: [PATCH 05/13] Serialise cart updates - do not submit another until the previous has completed --- .../darkswarm/services/cart.js.coffee | 23 ++++++- .../darkswarm/services/cart_spec.js.coffee | 68 +++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/darkswarm/services/cart.js.coffee b/app/assets/javascripts/darkswarm/services/cart.js.coffee index ed7d9ac273..f1c9421a15 100644 --- a/app/assets/javascripts/darkswarm/services/cart.js.coffee +++ b/app/assets/javascripts/darkswarm/services/cart.js.coffee @@ -2,8 +2,11 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, storage)-> # Handles syncing of current cart/order state to server new class Cart dirty: false + update_running: false + update_enqueued: false order: CurrentOrder.order line_items: CurrentOrder.order?.line_items || [] + constructor: -> for line_item in @line_items line_item.variant.line_item = line_item @@ -12,15 +15,31 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, storage)-> orderChanged: => @unsaved() + + if !@update_running + @scheduleUpdate() + else + @update_enqueued = true + + scheduleUpdate: => if @promise $timeout.cancel(@promise) @promise = $timeout @update, 1000 update: => + @update_running = true $http.post('/orders/populate', @data()).success (data, status)=> @saved() + @update_running = false + @popQueue() if @update_enqueued + .error (response, status)=> - @scheduleRetry() + @scheduleRetry(status) + @update_running = false + + popQueue: => + @update_enqueued = false + @scheduleUpdate() data: => variants = {} @@ -30,7 +49,7 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, storage)-> max_quantity: li.max_quantity {variants: variants} - scheduleRetry: => + scheduleRetry: (status) => console.log "Error updating cart: #{status}. Retrying in 3 seconds..." $timeout => console.log "Retrying cart update" diff --git a/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee index 0519b59763..518a524010 100644 --- a/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee @@ -48,9 +48,52 @@ describe 'Cart service', -> order.line_items[0].quantity = 2 expect(Cart.total_item_count()).toEqual 2 + describe "triggering cart updates", -> + it "schedules an update when there's no update running", -> + Cart.update_running = false + Cart.update_enqueued = false + spyOn(Cart, 'scheduleUpdate') + spyOn(Cart, 'unsaved') + Cart.orderChanged() + expect(Cart.scheduleUpdate).toHaveBeenCalled() + + it "enqueues an update when there's already an update running", -> + Cart.update_running = true + Cart.update_enqueued = false + spyOn(Cart, 'scheduleUpdate') + spyOn(Cart, 'unsaved') + Cart.orderChanged() + expect(Cart.scheduleUpdate).not.toHaveBeenCalled() + expect(Cart.update_enqueued).toBe(true) + + it "does nothing when there's already an update enqueued", -> + Cart.update_running = true + Cart.update_enqueued = true + spyOn(Cart, 'scheduleUpdate') + spyOn(Cart, 'unsaved') + Cart.orderChanged() + expect(Cart.scheduleUpdate).not.toHaveBeenCalled() + expect(Cart.update_enqueued).toBe(true) + describe "updating the cart", -> data = {variants: {}} + it "sets update_running during the update, and clears it on success", -> + $httpBackend.expectPOST("/orders/populate", data).respond 200, {} + expect(Cart.update_running).toBe(false) + Cart.update() + expect(Cart.update_running).toBe(true) + $httpBackend.flush() + expect(Cart.update_running).toBe(false) + + it "sets update_running during the update, and clears it on failure", -> + $httpBackend.expectPOST("/orders/populate", data).respond 404, {} + expect(Cart.update_running).toBe(false) + Cart.update() + expect(Cart.update_running).toBe(true) + $httpBackend.flush() + expect(Cart.update_running).toBe(false) + it "marks the form as saved on success", -> spyOn(Cart, 'saved') $httpBackend.expectPOST("/orders/populate", data).respond 200, {} @@ -58,6 +101,24 @@ describe 'Cart service', -> $httpBackend.flush() expect(Cart.saved).toHaveBeenCalled() + it "runs enqueued updates after success", -> + Cart.update_enqueued = true + spyOn(Cart, 'saved') + spyOn(Cart, 'popQueue') + $httpBackend.expectPOST("/orders/populate", data).respond 200, {} + Cart.update() + $httpBackend.flush() + expect(Cart.popQueue).toHaveBeenCalled() + + it "doesn't run an update if it's not enqueued", -> + Cart.update_enqueued = false + spyOn(Cart, 'saved') + spyOn(Cart, 'popQueue') + $httpBackend.expectPOST("/orders/populate", data).respond 200, {} + Cart.update() + $httpBackend.flush() + expect(Cart.popQueue).not.toHaveBeenCalled() + it "retries the update on failure", -> spyOn(Cart, 'scheduleRetry') $httpBackend.expectPOST("/orders/populate", data).respond 404, {} @@ -65,6 +126,13 @@ describe 'Cart service', -> $httpBackend.flush() expect(Cart.scheduleRetry).toHaveBeenCalled() + it "pops the queue", -> + Cart.update_enqueued = true + spyOn(Cart, 'scheduleUpdate') + Cart.popQueue() + expect(Cart.update_enqueued).toBe(false) + expect(Cart.scheduleUpdate).toHaveBeenCalled() + it "schedules retries of updates", -> spyOn(Cart, 'orderChanged') Cart.scheduleRetry() From 839bf8794f6c67c535219371a21a811170d69ac7 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 7 Jan 2016 07:06:26 +0100 Subject: [PATCH 06/13] Fix a hard cocded string on Order summary screen Created an order_pickup_time entry in locale files. --- app/views/spree/shared/_order_details.html.haml | 2 +- config/locales/en.yml | 1 + config/locales/fr.yml | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/spree/shared/_order_details.html.haml b/app/views/spree/shared/_order_details.html.haml index 25582aac8f..8be8d3cf50 100644 --- a/app/views/spree/shared/_order_details.html.haml +++ b/app/views/spree/shared/_order_details.html.haml @@ -62,7 +62,7 @@ %strong= order.shipping_method.name .pad .text-big - Ready for collection + = t :order_pickup_time %strong #{order.order_cycle.pickup_time_for(order.distributor)} %p.text-small.text-skinny.pre-line %em= order.shipping_method.description.andand.html_safe || "" diff --git a/config/locales/en.yml b/config/locales/en.yml index 04d350153b..701d758891 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -227,6 +227,7 @@ en: order_delivery_on: Delivery on order_delivery_address: Delivery address order_special_instructions: "Your notes:" + order_pickup_time: Ready for collection order_pickup_instructions: Collection Instructions order_produce: Produce order_total_price: Total diff --git a/config/locales/fr.yml b/config/locales/fr.yml index b0613efff4..7ddd01655a 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -166,6 +166,7 @@ fr: order_delivery_on: Livraison prévue order_delivery_address: Adresse de livraison order_special_instructions: "Vos commentaires:" + order_pickup_time: Disponible pour retrait order_pickup_instructions: Instructions de retrait order_produce: Produit order_total_price: Total From 01bf64e6b03658424b9cd547dc6dc2cccdda669f Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 7 Jan 2016 07:15:18 +0100 Subject: [PATCH 07/13] Allow delayed_job to use the locale defined for application delayed_job requires a different configuration variable for locale than the application. See https://stackoverflow.com/questions/8478597/rails-3-set-i18n-locale-is-not-working for reference --- config/application.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 8a492f5121..55bf36ad86 100644 --- a/config/application.rb +++ b/config/application.rb @@ -66,7 +66,8 @@ module Openfoodnetwork # The default locale is :en and all translations from config/locales/*.rb,yml are auto loaded. # config.i18n.load_path += Dir[Rails.root.join('my', 'locales', '*.{rb,yml}').to_s] config.i18n.default_locale = ENV["LOCALE"] - + I18n.locale = config.i18n.locale = config.i18n.default_locale + # Setting this to true causes a performance regression in Rails 3.2.17 # When we're on a version with the fix below, we can set it to true # https://github.com/svenfuchs/i18n/issues/230 From 770a8d0b17b6f191b87363694919646e13ddf6b4 Mon Sep 17 00:00:00 2001 From: Nicolas Blanc Date: Sun, 10 Jan 2016 09:54:35 +0100 Subject: [PATCH 08/13] #771-minor-bug-on-Firefox Z-index:1 solved problem on Firefox. It works on Chrome too. --- app/assets/stylesheets/darkswarm/_shop-product-thumb.css.sass | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/darkswarm/_shop-product-thumb.css.sass b/app/assets/stylesheets/darkswarm/_shop-product-thumb.css.sass index b5dca200c5..701e8005b0 100644 --- a/app/assets/stylesheets/darkswarm/_shop-product-thumb.css.sass +++ b/app/assets/stylesheets/darkswarm/_shop-product-thumb.css.sass @@ -13,7 +13,7 @@ height: 7rem float: left display: block - z-index: 999999 + z-index: 1 background-color: white overflow: hidden i @@ -56,4 +56,4 @@ width: 0rem height: 0rem - \ No newline at end of file + From 749061d60a994e093b84dd0a1af2ffb008d3f6a0 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 8 Jan 2016 18:42:03 +1100 Subject: [PATCH 09/13] knapsack report generation --- .travis.yml | 13 +++++++------ Gemfile | 1 + Gemfile.lock | 7 +++++++ Rakefile | 2 ++ spec/spec_helper.rb | 6 ++++++ 5 files changed, 23 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 952804f011..5867e4e5fe 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,11 +15,12 @@ env: - TZ="Australia/Melbourne" - TIMEZONE="Australia/Melbourne" matrix: - - TEST_CASES="./spec/features/admin" GITHUB_DEPLOY="true" - - TEST_CASES="./spec/features/consumer ./spec/serializers ./spec/performance" - - TEST_CASES="./spec/models" - - TEST_CASES="./spec/controllers ./spec/views ./spec/jobs" - - TEST_CASES="./spec/requests ./spec/helpers ./spec/mailers ./spec/lib" KARMA="true" + - TEST_CASES="./spec" +# - TEST_CASES="./spec/features/admin" GITHUB_DEPLOY="true" +# - TEST_CASES="./spec/features/consumer ./spec/serializers ./spec/performance" +# - TEST_CASES="./spec/models" +# - TEST_CASES="./spec/controllers ./spec/views ./spec/jobs" +# - TEST_CASES="./spec/requests ./spec/helpers ./spec/mailers ./spec/lib" KARMA="true" before_script: - cp config/database.travis.yml config/database.yml @@ -36,7 +37,7 @@ before_script: script: - '[ "$KARMA" = "true" ] && bundle exec rake karma:run || echo "Skipping karma run"' - - "bundle exec rspec $TEST_CASES" + - "KNAPSACK_GENERATE_REPORT=true bundle exec rspec $TEST_CASES" after_success: - > diff --git a/Gemfile b/Gemfile index 00fd2c8908..8230c6a15f 100644 --- a/Gemfile +++ b/Gemfile @@ -105,6 +105,7 @@ group :test, :development do gem 'json_spec' gem 'unicorn-rails' gem 'atomic' + gem 'knapsack' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index bbcbfde415..16a319f0ab 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -431,6 +431,9 @@ GEM actionpack (>= 3.0.0) activesupport (>= 3.0.0) kgio (2.9.3) + knapsack (1.5.1) + rake + timecop (>= 0.1.0) launchy (2.1.2) addressable (~> 2.3) letter_opener (1.0.0) @@ -684,6 +687,7 @@ DEPENDENCIES immigrant jquery-rails json_spec + knapsack letter_opener momentjs-rails newrelic_rpm @@ -726,3 +730,6 @@ DEPENDENCIES whenever wicked_pdf wkhtmltopdf-binary + +BUNDLED WITH + 1.10.6 diff --git a/Rakefile b/Rakefile index 699faf6e9d..9ed55e022e 100644 --- a/Rakefile +++ b/Rakefile @@ -5,3 +5,5 @@ require File.expand_path('../config/application', __FILE__) Openfoodnetwork::Application.load_tasks + +Knapsack.load_tasks if defined?(Knapsack) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 61552c528d..b398eba25c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,9 @@ +require 'knapsack' + +# CUSTOM_CONFIG_GOES_HERE + +Knapsack::Adapters::RSpecAdapter.bind + require 'rubygems' # Require pry when we're not inside Travis-CI From 7a998663b26a46fadb211fedabb485e7f8c5199b Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 8 Jan 2016 19:21:51 +1100 Subject: [PATCH 10/13] Knapsack report and test splitting --- .travis.yml | 15 ++-- knapsack_rspec_report.json | 173 +++++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 7 deletions(-) create mode 100644 knapsack_rspec_report.json diff --git a/.travis.yml b/.travis.yml index 5867e4e5fe..7764342b15 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,13 +14,13 @@ env: global: - TZ="Australia/Melbourne" - TIMEZONE="Australia/Melbourne" + - CI_NODE_TOTAL=5 matrix: - - TEST_CASES="./spec" -# - TEST_CASES="./spec/features/admin" GITHUB_DEPLOY="true" -# - TEST_CASES="./spec/features/consumer ./spec/serializers ./spec/performance" -# - TEST_CASES="./spec/models" -# - TEST_CASES="./spec/controllers ./spec/views ./spec/jobs" -# - TEST_CASES="./spec/requests ./spec/helpers ./spec/mailers ./spec/lib" KARMA="true" + - CI_NODE_INDEX=0 + - CI_NODE_INDEX=1 + - CI_NODE_INDEX=2 + - CI_NODE_INDEX=3 + - CI_NODE_INDEX=4 KARMA="true" GITHUB_DEPLOY="true" before_script: - cp config/database.travis.yml config/database.yml @@ -37,7 +37,8 @@ before_script: script: - '[ "$KARMA" = "true" ] && bundle exec rake karma:run || echo "Skipping karma run"' - - "KNAPSACK_GENERATE_REPORT=true bundle exec rspec $TEST_CASES" + #- "KNAPSACK_GENERATE_REPORT=true bundle exec rspec spec" + - "bundle exec rake knapsack:rspec" after_success: - > diff --git a/knapsack_rspec_report.json b/knapsack_rspec_report.json new file mode 100644 index 0000000000..9578580ffd --- /dev/null +++ b/knapsack_rspec_report.json @@ -0,0 +1,173 @@ +{ + "spec/controllers/admin/accounts_and_billing_settings_controller_spec.rb": 5.547292709350586, + "spec/controllers/admin/business_model_configuration_controller_spec.rb": 0.3683593273162842, + "spec/controllers/admin/customers_controller_spec.rb": 0.8933048248291016, + "spec/controllers/admin/enterprises_controller_spec.rb": 5.984264850616455, + "spec/controllers/admin/order_cycles_controller_spec.rb": 2.839667558670044, + "spec/controllers/api/enterprises_controller_spec.rb": 0.2780017852783203, + "spec/controllers/api/order_cycles_controller_spec.rb": 1.8730568885803223, + "spec/controllers/base_controller_spec.rb": 0.02932429313659668, + "spec/controllers/cart_controller_spec.rb": 1.062530517578125, + "spec/controllers/checkout_controller_spec.rb": 1.6658811569213867, + "spec/controllers/enterprise_confirmations_controller_spec.rb": 1.1228001117706299, + "spec/controllers/enterprises_controller_spec.rb": 2.2625372409820557, + "spec/controllers/groups_controller_spec.rb": 0.40616846084594727, + "spec/controllers/registration_controller_spec.rb": 0.2145981788635254, + "spec/controllers/shop_controller_spec.rb": 5.298644304275513, + "spec/controllers/shops_controller_spec.rb": 0.2002561092376709, + "spec/controllers/spree/admin/adjustments_controller_spec.rb": 1.023233413696289, + "spec/controllers/spree/admin/base_controller_spec.rb": 0.28871917724609375, + "spec/controllers/spree/admin/line_items_controller_spec.rb": 14.042466402053833, + "spec/controllers/spree/admin/orders_controller_spec.rb": 12.639750480651855, + "spec/controllers/spree/admin/overview_controller_spec.rb": 0.691641092300415, + "spec/controllers/spree/admin/payment_methods_controller_spec.rb": 0.7098217010498047, + "spec/controllers/spree/admin/products_controller_spec.rb": 1.4383087158203125, + "spec/controllers/spree/admin/reports_controller_spec.rb": 47.79633665084839, + "spec/controllers/spree/admin/search_controller_spec.rb": 0.9386723041534424, + "spec/controllers/spree/admin/variants_controller_spec.rb": 2.0663084983825684, + "spec/controllers/spree/api/line_items_controller_spec.rb": 0.4743325710296631, + "spec/controllers/spree/api/products_controller_spec.rb": 8.339523792266846, + "spec/controllers/spree/api/variants_controller_spec.rb": 4.835069179534912, + "spec/controllers/spree/checkout_controller_spec.rb": 0.687798023223877, + "spec/controllers/spree/orders_controller_spec.rb": 1.7623963356018066, + "spec/controllers/spree/paypal_controller_spec.rb": 0.437147855758667, + "spec/controllers/spree/store_controller_spec.rb": 0.03699040412902832, + "spec/controllers/spree/user_sessions_controller_spec.rb": 0.09967947006225586, + "spec/controllers/user_passwords_controller_spec.rb": 0.31070899963378906, + "spec/controllers/user_registrations_controller_spec.rb": 0.36581993103027344, + "spec/features/admin/account_spec.rb": 0.32449865341186523, + "spec/features/admin/accounts_and_billing_settings_spec.rb": 15.864763259887695, + "spec/features/admin/adjustments_spec.rb": 6.825028896331787, + "spec/features/admin/authentication_spec.rb": 22.29801869392395, + "spec/features/admin/bulk_order_management_spec.rb": 112.38913011550903, + "spec/features/admin/bulk_product_update_spec.rb": 59.00568914413452, + "spec/features/admin/business_model_configuration_spec.rb": 2.5152199268341064, + "spec/features/admin/cms_spec.rb": 2.5085999965667725, + "spec/features/admin/content_spec.rb": 1.2907540798187256, + "spec/features/admin/customers_spec.rb": 33.99929761886597, + "spec/features/admin/enterprise_fees_spec.rb": 13.33712100982666, + "spec/features/admin/enterprise_groups_spec.rb": 8.689672231674194, + "spec/features/admin/enterprise_relationships_spec.rb": 7.257282733917236, + "spec/features/admin/enterprise_roles_spec.rb": 5.535412788391113, + "spec/features/admin/enterprise_user_spec.rb": 2.5493221282958984, + "spec/features/admin/enterprises/index_spec.rb": 5.77092719078064, + "spec/features/admin/enterprises_spec.rb": 34.78606820106506, + "spec/features/admin/image_settings_spec.rb": 0.4501008987426758, + "spec/features/admin/order_cycles_spec.rb": 64.186044216156, + "spec/features/admin/orders_spec.rb": 49.190918922424316, + "spec/features/admin/overview_spec.rb": 5.788672208786011, + "spec/features/admin/payment_method_spec.rb": 15.959310531616211, + "spec/features/admin/products_spec.rb": 21.46337914466858, + "spec/features/admin/reports_spec.rb": 150.51152086257935, + "spec/features/admin/shipping_methods_spec.rb": 8.671862363815308, + "spec/features/admin/tax_settings_spec.rb": 0.7941949367523193, + "spec/features/admin/variant_overrides_spec.rb": 29.70982050895691, + "spec/features/admin/variants_spec.rb": 5.565031290054321, + "spec/features/consumer/authentication_spec.rb": 12.449390649795532, + "spec/features/consumer/groups_spec.rb": 1.545715093612671, + "spec/features/consumer/producers_spec.rb": 3.3242862224578857, + "spec/features/consumer/registration_spec.rb": 2.421873092651367, + "spec/features/consumer/shopping/cart_spec.rb": 1.6924467086791992, + "spec/features/consumer/shopping/checkout_auth_spec.rb": 8.496914863586426, + "spec/features/consumer/shopping/checkout_spec.rb": 39.204933881759644, + "spec/features/consumer/shopping/shopping_spec.rb": 23.358332633972168, + "spec/features/consumer/shopping/variant_overrides_spec.rb": 58.16736888885498, + "spec/features/consumer/shops_spec.rb": 6.636866092681885, + "spec/helpers/admin/business_model_configuration_helper_spec.rb": 0.2595028877258301, + "spec/helpers/checkout_helper_spec.rb": 0.10617446899414062, + "spec/helpers/groups_helper_spec.rb": 0.007729053497314453, + "spec/helpers/html_helper_spec.rb": 0.05157279968261719, + "spec/helpers/injection_helper_spec.rb": 0.6142556667327881, + "spec/helpers/navigation_helper_spec.rb": 0.02951979637145996, + "spec/helpers/order_cycles_helper_spec.rb": 0.5953588485717773, + "spec/helpers/products_helper_spec.rb": 0.009511232376098633, + "spec/helpers/shared_helper_spec.rb": 0.017564058303833008, + "spec/helpers/shop_helper_spec.rb": 0.05760025978088379, + "spec/jobs/confirm_order_job_spec.rb": 0.0458524227142334, + "spec/jobs/confirm_signup_job_spec.rb": 0.021564006805419922, + "spec/jobs/finalize_account_invoices_spec.rb": 4.505181312561035, + "spec/jobs/order_cycle_notification_job_spec.rb": 2.0606272220611572, + "spec/jobs/update_account_invoices_spec.rb": 18.434475898742676, + "spec/jobs/update_billable_periods_spec.rb": 4.850176572799683, + "spec/jobs/welcome_enterprise_job_spec.rb": 0.07065534591674805, + "spec/lib/open_food_network/bulk_coop_report_spec.rb": 4.789663553237915, + "spec/lib/open_food_network/customers_report_spec.rb": 2.419727325439453, + "spec/lib/open_food_network/distribution_change_validator_spec.rb": 0.10607743263244629, + "spec/lib/open_food_network/enterprise_fee_applicator_spec.rb": 0.7333858013153076, + "spec/lib/open_food_network/enterprise_fee_calculator_spec.rb": 7.406745195388794, + "spec/lib/open_food_network/enterprise_injection_data_spec.rb": 0.291548490524292, + "spec/lib/open_food_network/enterprise_issue_validator_spec.rb": 0.09764814376831055, + "spec/lib/open_food_network/feature_toggle_spec.rb": 0.010193109512329102, + "spec/lib/open_food_network/group_buy_report_spec.rb": 3.708569049835205, + "spec/lib/open_food_network/last_used_address_spec.rb": 0.0254666805267334, + "spec/lib/open_food_network/lettuce_share_report_spec.rb": 2.3206725120544434, + "spec/lib/open_food_network/option_value_namer_spec.rb": 0.06185555458068848, + "spec/lib/open_food_network/order_and_distributor_report_spec.rb": 1.0406858921051025, + "spec/lib/open_food_network/order_cycle_form_applicator_spec.rb": 4.533008337020874, + "spec/lib/open_food_network/order_cycle_management_report_spec.rb": 2.036308526992798, + "spec/lib/open_food_network/order_cycle_permissions_spec.rb": 23.74185061454773, + "spec/lib/open_food_network/order_grouper_spec.rb": 0.029039621353149414, + "spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb": 5.135573148727417, + "spec/lib/open_food_network/packing_report_spec.rb": 5.088447093963623, + "spec/lib/open_food_network/permissions_spec.rb": 8.881855249404907, + "spec/lib/open_food_network/products_and_inventory_report_spec.rb": 3.55375337600708, + "spec/lib/open_food_network/referer_parser_spec.rb": 0.014271259307861328, + "spec/lib/open_food_network/reports/report_spec.rb": 0.02238297462463379, + "spec/lib/open_food_network/reports/row_spec.rb": 0.0031762123107910156, + "spec/lib/open_food_network/reports/rule_spec.rb": 0.013959169387817383, + "spec/lib/open_food_network/sales_tax_report_spec.rb": 0.10717129707336426, + "spec/lib/open_food_network/scope_variant_to_hub_spec.rb": 2.4846229553222656, + "spec/lib/open_food_network/user_balance_calculator_spec.rb": 3.4277901649475098, + "spec/lib/open_food_network/users_and_enterprises_report_spec.rb": 0.40532779693603516, + "spec/lib/open_food_network/xero_invoices_report_spec.rb": 1.1586685180664062, + "spec/lib/spree/product_filters_spec.rb": 0.13163042068481445, + "spec/mailers/enterprise_mailer_spec.rb": 0.4537942409515381, + "spec/mailers/order_mailer_spec.rb": 1.452355146408081, + "spec/mailers/producer_mailer_spec.rb": 8.775528192520142, + "spec/mailers/user_mailer_spec.rb": 0.057527780532836914, + "spec/models/adjustment_metadata_spec.rb": 0.22016620635986328, + "spec/models/billable_period_spec.rb": 2.06524658203125, + "spec/models/calculator/weight_spec.rb": 0.009344100952148438, + "spec/models/cart_spec.rb": 4.099429130554199, + "spec/models/customer_spec.rb": 0.07328605651855469, + "spec/models/enterprise_caching_spec.rb": 0.8475983142852783, + "spec/models/enterprise_fee_spec.rb": 3.1999905109405518, + "spec/models/enterprise_group_spec.rb": 0.30861926078796387, + "spec/models/enterprise_relationship_spec.rb": 2.1849746704101562, + "spec/models/enterprise_spec.rb": 17.679611682891846, + "spec/models/exchange_spec.rb": 13.899227857589722, + "spec/models/model_set_spec.rb": 0.22760748863220215, + "spec/models/order_cycle_spec.rb": 10.680967569351196, + "spec/models/product_distribution_spec.rb": 2.227938413619995, + "spec/models/spree/ability_spec.rb": 15.278357028961182, + "spec/models/spree/addresses_spec.rb": 0.055602312088012695, + "spec/models/spree/adjustment_spec.rb": 9.196375846862793, + "spec/models/spree/classification_spec.rb": 0.161299467086792, + "spec/models/spree/image_spec.rb": 0.007464408874511719, + "spec/models/spree/line_item_spec.rb": 13.545411586761475, + "spec/models/spree/order_populator_spec.rb": 1.635932207107544, + "spec/models/spree/order_spec.rb": 10.645411968231201, + "spec/models/spree/payment_method_spec.rb": 0.0733034610748291, + "spec/models/spree/payment_spec.rb": 1.691227912902832, + "spec/models/spree/preferences/file_configuration_spec.rb": 0.03429675102233887, + "spec/models/spree/product_spec.rb": 17.406191110610962, + "spec/models/spree/shipping_method_spec.rb": 3.0447566509246826, + "spec/models/spree/tax_rate_spec.rb": 0.44750261306762695, + "spec/models/spree/taxon_spec.rb": 0.553098201751709, + "spec/models/spree/user_spec.rb": 1.2693369388580322, + "spec/models/spree/variant_spec.rb": 13.75825023651123, + "spec/models/variant_override_spec.rb": 4.086935520172119, + "spec/performance/injection_helper_spec.rb": 6.890667676925659, + "spec/performance/orders_controller_spec.rb": 0.031180143356323242, + "spec/performance/shop_controller_spec.rb": 18.19426918029785, + "spec/requests/large_request_spec.rb": 0.02229022979736328, + "spec/requests/shop_spec.rb": 1.0012562274932861, + "spec/serializers/admin/enterprise_serializer_spec.rb": 0.10484433174133301, + "spec/serializers/admin/exchange_serializer_spec.rb": 0.7569985389709473, + "spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb": 0.4293792247772217, + "spec/serializers/admin/index_enterprise_serializer_spec.rb": 1.2506742477416992, + "spec/serializers/admin/variant_override_serializer_spec.rb": 0.38981151580810547, + "spec/serializers/enterprise_serializer_spec.rb": 0.3511006832122803, + "spec/serializers/spree/product_serializer_spec.rb": 0.26622653007507324, + "spec/serializers/spree/variant_serializer_spec.rb": 0.30304574966430664 +} From 00af6ef9deb8bc4f7eca233276c3c40e554a2b26 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 13 Jan 2016 12:11:42 +1100 Subject: [PATCH 11/13] cleanup knapsack integration --- spec/spec_helper.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b398eba25c..80d7c2eeb7 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,13 +1,10 @@ -require 'knapsack' - -# CUSTOM_CONFIG_GOES_HERE - -Knapsack::Adapters::RSpecAdapter.bind - require 'rubygems' # Require pry when we're not inside Travis-CI -require 'pry' unless ENV['HAS_JOSH_K_SEAL_OF_APPROVAL'] +require 'pry' unless ENV['CI'] + +require 'knapsack' +Knapsack::Adapters::RSpecAdapter.bind ENV["RAILS_ENV"] ||= 'test' require File.expand_path("../../config/environment", __FILE__) From 0c434c197b5106dae53f4345c5af6ab84153f6c3 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 13 Jan 2016 16:07:04 +1100 Subject: [PATCH 12/13] Making Travis fail if karma fails --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 7764342b15..0ba99dc9e1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -36,7 +36,7 @@ before_script: fi script: - - '[ "$KARMA" = "true" ] && bundle exec rake karma:run || echo "Skipping karma run"' + - 'if [ "$KARMA" = "true" ]; then bundle exec rake karma:run; else echo "Skipping karma run"; fi' #- "KNAPSACK_GENERATE_REPORT=true bundle exec rspec spec" - "bundle exec rake knapsack:rspec" From c33835e75151fa64c226220b855cc017db51f7e0 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 13 Jan 2016 16:23:14 +1100 Subject: [PATCH 13/13] fixup whitespace --- config/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 55bf36ad86..ba75a097ec 100644 --- a/config/application.rb +++ b/config/application.rb @@ -67,7 +67,7 @@ module Openfoodnetwork # config.i18n.load_path += Dir[Rails.root.join('my', 'locales', '*.{rb,yml}').to_s] config.i18n.default_locale = ENV["LOCALE"] I18n.locale = config.i18n.locale = config.i18n.default_locale - + # Setting this to true causes a performance regression in Rails 3.2.17 # When we're on a version with the fix below, we can set it to true # https://github.com/svenfuchs/i18n/issues/230