From 082a3cd9abb86e1bf049f7775f7cbd081d13e8c1 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Nov 2014 12:29:47 +1100 Subject: [PATCH 1/8] Creating simple_order_cycle instead of order_cycle Speedup on my machine: 1 minute 44.52 seconds 21.9 seconds --- spec/controllers/checkout_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index dcbd1d5177..0499febfc9 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe CheckoutController do let(:distributor) { double(:distributor) } - let(:order_cycle) { create(:order_cycle) } + let(:order_cycle) { create(:simple_order_cycle) } let(:order) { create(:order) } before do order.stub(:checkout_allowed?).and_return true From 7f764db4d72e1a2e91e795b0a47087e9a12e484b Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Nov 2014 12:39:05 +1100 Subject: [PATCH 2/8] Using simple_order_cycle order_cycle: 1 minute 56.88 seconds simple_order_cycle: 1 minute 8.05 seconds --- spec/features/consumer/home_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/consumer/home_spec.rb b/spec/features/consumer/home_spec.rb index d08dee5dfc..f5b8091432 100644 --- a/spec/features/consumer/home_spec.rb +++ b/spec/features/consumer/home_spec.rb @@ -8,7 +8,7 @@ feature 'Home', js: true do let!(:invisible_distributor) { create(:distributor_enterprise, visible: false) } let(:d1) { create(:distributor_enterprise) } let(:d2) { create(:distributor_enterprise) } - let!(:order_cycle) { create(:order_cycle, distributors: [distributor], coordinator: create(:distributor_enterprise)) } + let!(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], coordinator: create(:distributor_enterprise)) } let!(:producer) { create(:supplier_enterprise) } let!(:er) { create(:enterprise_relationship, parent: distributor, child: producer) } From ccd03bfa8479ff4c831a3e7e6e13d5d53c583fa5 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Nov 2014 13:20:05 +1100 Subject: [PATCH 3/8] Using simple_order_cycle Test timings were not accurate. Output: 4 order_cycles: 9.94 seconds 2 order_cycles: 9.87 seconds 0 order_cycles: 9.9 seconds Felt execution was much higher --- spec/models/spree/product_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index c6acae0850..f9a990830c 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -359,8 +359,8 @@ module Spree d2 = create(:distributor_enterprise) p1 = create(:product) p2 = create(:product) - oc1 = create(:order_cycle, :distributors => [d1], :variants => [p1.master]) - oc2 = create(:order_cycle, :distributors => [d2], :variants => [p2.master]) + oc1 = create(:simple_order_cycle, :distributors => [d1], :variants => [p1.master]) + oc2 = create(:simple_order_cycle, :distributors => [d2], :variants => [p2.master]) p1.should be_in_distributor d1 p1.should_not be_in_distributor d2 @@ -371,8 +371,8 @@ module Spree d2 = create(:distributor_enterprise) p1 = create(:product) p2 = create(:product) - oc1 = create(:order_cycle, :distributors => [d1], :variants => [p1.master]) - oc2 = create(:order_cycle, :distributors => [d2], :variants => [p2.master]) + oc1 = create(:simple_order_cycle, :distributors => [d1], :variants => [p1.master]) + oc2 = create(:simple_order_cycle, :distributors => [d2], :variants => [p2.master]) p1.should be_in_order_cycle oc1 p1.should_not be_in_order_cycle oc2 From aa2cefb88cebe366af8de37f7a981d1166443730 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Nov 2014 13:45:25 +1100 Subject: [PATCH 4/8] Using simple_order_cycle where applicable Before: 3 minutes 0 seconds After: 1 minute 21.02 seconds --- spec/models/order_cycle_spec.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 56aa51a0d0..f436690850 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -91,25 +91,25 @@ describe OrderCycle do end it "finds the most recently closed order cycles" do - oc1 = create(:order_cycle, orders_close_at: 2.hours.ago) - oc2 = create(:order_cycle, orders_close_at: 1.hour.ago) - oc3 = create(:order_cycle, orders_close_at: 1.hour.from_now) + oc1 = create(:simple_order_cycle, orders_close_at: 2.hours.ago) + oc2 = create(:simple_order_cycle, orders_close_at: 1.hour.ago) + oc3 = create(:simple_order_cycle, orders_close_at: 1.hour.from_now) OrderCycle.most_recently_closed.should == [oc2, oc1] end it "finds the soonest opening order cycles" do - oc1 = create(:order_cycle, orders_open_at: 1.weeks.from_now) - oc2 = create(:order_cycle, orders_open_at: 2.hours.from_now) - oc3 = create(:order_cycle, orders_open_at: 1.hour.ago) + oc1 = create(:simple_order_cycle, orders_open_at: 1.weeks.from_now) + oc2 = create(:simple_order_cycle, orders_open_at: 2.hours.from_now) + oc3 = create(:simple_order_cycle, orders_open_at: 1.hour.ago) OrderCycle.soonest_opening.should == [oc2, oc1] end it "finds the soonest closing order cycles" do - oc1 = create(:order_cycle, orders_close_at: 2.hours.ago) - oc2 = create(:order_cycle, orders_close_at: 2.hour.from_now) - oc3 = create(:order_cycle, orders_close_at: 1.hour.from_now) + oc1 = create(:simple_order_cycle, orders_close_at: 2.hours.ago) + oc2 = create(:simple_order_cycle, orders_close_at: 2.hour.from_now) + oc3 = create(:simple_order_cycle, orders_close_at: 1.hour.from_now) OrderCycle.soonest_closing.should == [oc3, oc2] end @@ -311,7 +311,7 @@ describe OrderCycle do end describe "checking status" do - let(:oc) { create(:order_cycle) } + let(:oc) { create(:simple_order_cycle) } it "reports status when an order cycle is upcoming" do Timecop.freeze(oc.orders_open_at - 1.second) do From 73b8f37d77266214ff96c5edbd6057049212883e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Nov 2014 13:55:06 +1100 Subject: [PATCH 5/8] Using simple_order_cycle Before: 2 minutes 8.7 seconds After: 16.11 seconds --- spec/controllers/api/order_cycles_controller_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/controllers/api/order_cycles_controller_spec.rb b/spec/controllers/api/order_cycles_controller_spec.rb index 7f8bbec7af..a9cceac796 100644 --- a/spec/controllers/api/order_cycles_controller_spec.rb +++ b/spec/controllers/api/order_cycles_controller_spec.rb @@ -6,8 +6,8 @@ module Api include Spree::Api::TestingSupport::Helpers render_views - let!(:oc1) { FactoryGirl.create(:order_cycle) } - let!(:oc2) { FactoryGirl.create(:order_cycle) } + let!(:oc1) { FactoryGirl.create(:simple_order_cycle) } + let!(:oc2) { FactoryGirl.create(:simple_order_cycle) } let(:coordinator) { oc1.coordinator } let(:attributes) { [:id, :name, :suppliers, :distributors] } @@ -70,7 +70,7 @@ module Api user.save! user end - let!(:order_cycle) { create(:order_cycle, suppliers: [oc_supplier], distributors: [oc_distributor]) } + let!(:order_cycle) { create(:simple_order_cycle, suppliers: [oc_supplier], distributors: [oc_distributor]) } context "as the user of a supplier to an order cycle" do before :each do From 349b7de11a875382e4c76507a6a35b2d3fac9a85 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Nov 2014 14:02:55 +1100 Subject: [PATCH 6/8] Using simple_order_cycle Before: 2 minutes 58.3 seconds After: 35.04 seconds --- spec/controllers/shop_controller_spec.rb | 26 ++++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/spec/controllers/shop_controller_spec.rb b/spec/controllers/shop_controller_spec.rb index f1c132bb24..bfa68da912 100644 --- a/spec/controllers/shop_controller_spec.rb +++ b/spec/controllers/shop_controller_spec.rb @@ -16,21 +16,21 @@ describe ShopController do describe "Selecting order cycles" do it "should select an order cycle when only one order cycle is open" do - oc1 = create(:order_cycle, distributors: [d]) + oc1 = create(:simple_order_cycle, distributors: [d]) spree_get :show controller.current_order_cycle.should == oc1 end it "should not set an order cycle when multiple order cycles are open" do - oc1 = create(:order_cycle, distributors: [d]) - oc2 = create(:order_cycle, distributors: [d]) + oc1 = create(:simple_order_cycle, distributors: [d]) + oc2 = create(:simple_order_cycle, distributors: [d]) spree_get :show controller.current_order_cycle.should == nil end it "should allow the user to post to select the current order cycle" do - oc1 = create(:order_cycle, distributors: [d]) - oc2 = create(:order_cycle, distributors: [d]) + oc1 = create(:simple_order_cycle, distributors: [d]) + oc2 = create(:simple_order_cycle, distributors: [d]) spree_post :order_cycle, order_cycle_id: oc2.id response.should be_success @@ -40,8 +40,8 @@ describe ShopController do context "RABL tests" do render_views it "should return the order cycle details when the oc is selected" do - oc1 = create(:order_cycle, distributors: [d]) - oc2 = create(:order_cycle, distributors: [d]) + oc1 = create(:simple_order_cycle, distributors: [d]) + oc2 = create(:simple_order_cycle, distributors: [d]) spree_post :order_cycle, order_cycle_id: oc2.id response.should be_success @@ -49,7 +49,7 @@ describe ShopController do end it "should return the current order cycle when hit with GET" do - oc1 = create(:order_cycle, distributors: [d]) + oc1 = create(:simple_order_cycle, distributors: [d]) controller.stub(:current_order_cycle).and_return oc1 spree_get :order_cycle response.body.should have_content oc1.id @@ -57,9 +57,9 @@ describe ShopController do end it "should not allow the user to select an invalid order cycle" do - oc1 = create(:order_cycle, distributors: [d]) - oc2 = create(:order_cycle, distributors: [d]) - oc3 = create(:order_cycle, distributors: [create(:distributor_enterprise)]) + oc1 = create(:simple_order_cycle, distributors: [d]) + oc2 = create(:simple_order_cycle, distributors: [d]) + oc3 = create(:simple_order_cycle, distributors: [create(:distributor_enterprise)]) spree_post :order_cycle, order_cycle_id: oc3.id response.status.should == 404 @@ -71,7 +71,7 @@ describe ShopController do describe "producers/suppliers" do let(:supplier) { create(:supplier_enterprise) } let(:product) { create(:product, supplier: supplier) } - let(:order_cycle) { create(:order_cycle, distributors: [d], coordinator: create(:distributor_enterprise)) } + let(:order_cycle) { create(:simple_order_cycle, distributors: [d], coordinator: create(:distributor_enterprise)) } before do exchange = Exchange.find(order_cycle.exchanges.to_enterprises(d).outgoing.first.id) @@ -81,7 +81,7 @@ describe ShopController do describe "returning products" do let(:product) { create(:product) } - let(:order_cycle) { create(:order_cycle, distributors: [d], coordinator: create(:distributor_enterprise)) } + let(:order_cycle) { create(:simple_order_cycle, distributors: [d], coordinator: create(:distributor_enterprise)) } let(:exchange) { Exchange.find(order_cycle.exchanges.to_enterprises(d).outgoing.first.id) } before do From e240933b29ac0f1b9b12ed13049bfb47943ad1ef Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Nov 2014 14:45:35 +1100 Subject: [PATCH 7/8] Using simple_order_cycle in spec Before: 4 minutes 6.9 seconds After: 3 minutes 27.4 seconds --- spec/features/admin/order_cycles_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 3d310df1ba..180485a734 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -360,9 +360,9 @@ feature %q{ scenario "updating many order cycle opening/closing times at once" do # Given three order cycles - oc1 = create(:order_cycle) - oc2 = create(:order_cycle) - oc3 = create(:order_cycle) + oc1 = create(:simple_order_cycle) + oc2 = create(:simple_order_cycle) + oc3 = create(:simple_order_cycle) # When I go to the order cycles page login_to_admin_section @@ -394,7 +394,7 @@ feature %q{ scenario "cloning an order cycle" do # Given an order cycle - oc = create(:order_cycle) + oc = create(:simple_order_cycle) # When I clone it login_to_admin_section @@ -625,7 +625,7 @@ feature %q{ end it "shows me an index of order cycles without enterprise columns" do - create(:order_cycle, coordinator: enterprise) + create(:simple_order_cycle, coordinator: enterprise) visit admin_order_cycles_path page.should_not have_selector 'th', text: 'SUPPLIERS' page.should_not have_selector 'th', text: 'COORDINATOR' From 8e280919ac7fd5d9b240e68b8a1506592eb3022d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 28 Nov 2014 15:36:34 +1100 Subject: [PATCH 8/8] Using simple_order_cycle in clone spec Creating a coordinator fee and two exchanges in the spec instead of using a full order_cycle. Timing of this single test: Before: 15.32 seconds After: 6.26 seconds --- spec/models/order_cycle_spec.rb | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index f436690850..7ba2b66589 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -349,23 +349,30 @@ describe OrderCycle do end it "clones itself" do - oc = create(:order_cycle) - occ = oc.clone! + coordinator = create(:enterprise); + oc = create(:simple_order_cycle, coordinator_fees: [create(:enterprise_fee, enterprise: coordinator)]) + ex1 = create(:exchange, order_cycle: oc) + ex2 = create(:exchange, order_cycle: oc) + oc.clone! occ = OrderCycle.last occ.name.should == "COPY OF #{oc.name}" occ.orders_open_at.should be_nil occ.orders_close_at.should be_nil + occ.coordinator.should_not be_nil occ.coordinator.should == oc.coordinator + occ.coordinator_fee_ids.should_not be_empty occ.coordinator_fee_ids.should == oc.coordinator_fee_ids - #(0..occ.exchanges.count).all? { |i| occ.exchanges[i].eql? oc.exchanges[i] }.should be_true - # to_h gives us a unique hash for each exchange + # check that the clone has no additional exchanges occ.exchanges.map(&:to_h).all? do |ex| oc.exchanges.map(&:to_h).include? ex end + # check that the clone has original exchanges + occ.exchanges.map(&:to_h).include? ex1.to_h + occ.exchanges.map(&:to_h).include? ex2.to_h end describe "finding recently closed order cycles" do