From 10cd654ff58092f66080c36fbba781ddb6f47976 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 11 Sep 2015 15:41:44 +1000 Subject: [PATCH 1/4] CI Handling git tags Buildkite is running tags like branches. This caused `git branch` commands to fail. The function got extended to handle tags as well. Ideally, Buildkite will offer an option not to rebuild tags. --- script/ci/includes.sh | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/script/ci/includes.sh b/script/ci/includes.sh index a0599504c2..619d03153e 100644 --- a/script/ci/includes.sh +++ b/script/ci/includes.sh @@ -5,18 +5,33 @@ function load_environment { fi } +function master_merged { + if [[ `git tag -l "$BUILDKITE_BRANCH"` != '' ]]; then + echo "'$BUILDKITE_BRANCH' is a tag." + if [[ `git tag -l --contains origin/master "$BUILDKITE_BRANCH"` != '' ]]; then + echo "This tag contains the current master." + return 0 + else + echo "This tag does not contain the current master." + return 1 + fi + fi + if [[ `git branch -r --merged origin/$BUILDKITE_BRANCH` == *origin/master* ]]; then + echo "This branch already has the current master merged." + return 0 + fi + return 1 +} + function exit_unless_master_merged { - if [[ `git branch -a --merged origin/$BUILDKITE_BRANCH` != *origin/master* ]]; then + if ! master_merged; then echo "This branch does not have the current master merged. Please merge master and push again." exit 1 fi } function succeed_if_master_merged { - if [[ `git branch -a --merged origin/$BUILDKITE_BRANCH` == *origin/master* ]]; then - echo "This branch already has the current master merged." - exit 0 - fi + master_merged && exit 0 } function set_ofn_commit { From 3f822ed0e3b7bcac3ff9a8b200f08fb7cd4cc40d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 16 Sep 2015 13:06:32 +1000 Subject: [PATCH 2/4] trying to get rid of intermittent failures --- spec/features/admin/bulk_order_management_spec.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index bf77ab415a..ef77ef7a3c 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -124,7 +124,7 @@ feature %q{ visit '/admin/orders/bulk_management' end - pending "displays an update button which submits pending changes" do + it "displays an update button which submits pending changes" do fill_in "quantity", :with => 2 page.should have_selector "input[name='quantity'].update-pending" page.should_not have_selector "input[name='quantity'].update-success" @@ -132,6 +132,8 @@ feature %q{ click_button "Update" page.should_not have_selector "input[name='quantity'].update-pending" page.should have_selector "input[name='quantity'].update-success" + page.should have_selector "input[name='final_weight_volume'].update-success", visible: false + page.should have_selector "input[name='price'].update-success", visible: false end end end @@ -288,7 +290,7 @@ feature %q{ visit '/admin/orders/bulk_management' end - pending "displays a select box for order cycles, which filters line items by the selected order cycle" do + it "displays a select box for order cycles, which filters line items by the selected order cycle" do order_cycle_names = ["All"] OrderCycle.all.each{ |oc| order_cycle_names << oc.name } find("div.select2-container#s2id_order_cycle_filter").click @@ -440,7 +442,7 @@ feature %q{ page.should have_button "SAVE" end - pending "saves pendings changes when 'SAVE' button is clicked" do + it "saves pendings changes when 'SAVE' button is clicked" do within("tr#li_#{li2.id} td.quantity") do page.fill_in "quantity", :with => (li2.quantity + 1).to_s end @@ -450,6 +452,10 @@ feature %q{ within("tr#li_#{li2.id} td.quantity") do page.should have_field "quantity", :with => ( li2.quantity + 1 ).to_s end + page.should_not have_selector "input[name='quantity'].update-pending" + page.should_not have_selector "input[name='price'].update-pending" + page.should_not have_selector "input[name='final_weight_volume'].update-pending" + page.should have_no_selector "input.update-pending" end it "ignores pending changes when 'IGNORE' button is clicked" do From 2488411b943682f8842851eeff05c160e02c1c47 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 16 Sep 2015 14:24:01 +1000 Subject: [PATCH 3/4] JS feature specs completing before cleaning db Some specs ran into a deadlock when the Database::Cleaner tried to do its job while AJAX requests were still triggering other actions. --- Gemfile | 1 + Gemfile.lock | 2 + config/environments/test.rb | 5 ++ lib/open_food_network/rack_request_blocker.rb | 77 +++++++++++++++++++ .../admin/bulk_order_management_spec.rb | 11 +-- spec/spec_helper.rb | 5 ++ 6 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 lib/open_food_network/rack_request_blocker.rb diff --git a/Gemfile b/Gemfile index bdfd0b3410..c9d34cd0bb 100644 --- a/Gemfile +++ b/Gemfile @@ -99,6 +99,7 @@ group :test, :development do gem 'rspec-retry' gem 'json_spec' gem 'unicorn-rails' + gem 'atomic' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 2157de2b77..54f7daf530 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -155,6 +155,7 @@ GEM angularjs-rails (1.2.13) ansi (1.4.2) arel (3.0.3) + atomic (1.1.99) awesome_nested_set (2.1.5) activerecord (>= 3.0.0) awesome_print (1.0.2) @@ -547,6 +548,7 @@ DEPENDENCIES angular-rails-templates (~> 0.2.0) angularjs-file-upload-rails (~> 1.1.0) angularjs-rails (= 1.2.13) + atomic awesome_print aws-sdk blockenspiel diff --git a/config/environments/test.rb b/config/environments/test.rb index 46d5dcf7c6..67131b6489 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -41,6 +41,11 @@ Openfoodnetwork::Application.configure do # Print deprecation notices to the stderr config.active_support.deprecation = :stderr config.action_mailer.default_url_options = { :host => "test.host" } + + # To block requests before running the database cleaner + require 'open_food_network/rack_request_blocker' + # Make sure the middleware is inserted first in middleware chain + config.middleware.insert_before('ActionDispatch::Static', 'RackRequestBlocker') end # Allows us to use _url helpers in Rspec diff --git a/lib/open_food_network/rack_request_blocker.rb b/lib/open_food_network/rack_request_blocker.rb new file mode 100644 index 0000000000..71f1c450e2 --- /dev/null +++ b/lib/open_food_network/rack_request_blocker.rb @@ -0,0 +1,77 @@ +# Copied from http://blog.salsify.com/engineering/tearing-capybara-ajax-tests +# https://gist.github.com/jturkel/9317269/raw/ff7838684370fd8a468ffe1e5ce1f3e46ba39951/rack_request_blocker.rb + +require 'atomic' + +# Rack middleware that keeps track of the number of active requests and can block new requests. +class RackRequestBlocker + + @@num_active_requests = Atomic.new(0) + @@block_requests = Atomic.new(false) + + # Returns the number of requests the server is currently processing. + def self.num_active_requests + @@num_active_requests.value + end + + # Prevents the server from accepting new requests. Any new requests will return an HTTP + # 503 status. + def self.block_requests! + @@block_requests.value = true + end + + # Allows the server to accept requests again. + def self.allow_requests! + @@block_requests.value = false + end + + def initialize(app) + @app = app + end + + def call(env) + increment_active_requests + if block_requests? + block_request(env) + else + @app.call(env) + end + ensure + decrement_active_requests + end + + def self.wait_for_requests_complete + self.block_requests! + max_wait_time = 30 + polling_interval = 0.01 + wait_until = Time.now + max_wait_time.seconds + while true + return if self.num_active_requests == 0 + if Time.now > wait_until + raise "Failed waiting for completing requests, #{self.num_active_requests} running." + else + sleep(polling_interval) + end + end + ensure + self.allow_requests! + end + + private + + def block_requests? + @@block_requests.value + end + + def block_request(env) + [503, {}, []] + end + + def increment_active_requests + @@num_active_requests.update { |v| v + 1 } + end + + def decrement_active_requests + @@num_active_requests.update { |v| v - 1 } + end +end diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index ef77ef7a3c..8d52f21b48 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -291,8 +291,7 @@ feature %q{ end it "displays a select box for order cycles, which filters line items by the selected order cycle" do - order_cycle_names = ["All"] - OrderCycle.all.each{ |oc| order_cycle_names << oc.name } + order_cycle_names = OrderCycle.pluck(:name).push "All" find("div.select2-container#s2id_order_cycle_filter").click order_cycle_names.each { |ocn| page.should have_selector "div.select2-drop-active ul.select2-results li", text: ocn } find("div.select2-container#s2id_order_cycle_filter").click @@ -447,15 +446,13 @@ feature %q{ page.fill_in "quantity", :with => (li2.quantity + 1).to_s end fill_in "start_date_filter", :with => (Date.today - 9).strftime("%F %T") + page.should have_selector "input[name='quantity'].update-pending" click_button "SAVE" - page.should_not have_selector "input[name='quantity'].update-pending" + page.should have_no_selector "input.update-pending" + page.should have_selector "input[name='quantity'].update-success" within("tr#li_#{li2.id} td.quantity") do page.should have_field "quantity", :with => ( li2.quantity + 1 ).to_s end - page.should_not have_selector "input[name='quantity'].update-pending" - page.should_not have_selector "input[name='price'].update-pending" - page.should_not have_selector "input[name='final_weight_volume'].update-pending" - page.should have_no_selector "input.update-pending" end it "ignores pending changes when 'IGNORE' button is clicked" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 79d5c02d86..068a2184de 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -80,6 +80,11 @@ RSpec.configure do |config| config.before(:each, js: true) { DatabaseCleaner.strategy = :deletion, {except: ['spree_countries', 'spree_states']} } config.before(:each) { DatabaseCleaner.start } config.after(:each) { DatabaseCleaner.clean } + config.after(:each, js:true) do + Capybara.reset_sessions! + RackRequestBlocker.wait_for_requests_complete + DatabaseCleaner.clean + end # Geocoding config.before(:each) { Spree::Address.any_instance.stub(:geocode).and_return([1,1]) } From 637e5c4fee50da26ffeadec7ecfb79f2172e6370 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 9 Sep 2015 11:43:30 +1000 Subject: [PATCH 4/4] PermalinkGenerator for products --- app/models/enterprise_group.rb | 19 +++-------------- app/models/spree/product_decorator.rb | 12 +++++++++++ lib/open_food_network/permalink_generator.rb | 22 ++++++++++++++++++++ 3 files changed, 37 insertions(+), 16 deletions(-) create mode 100644 lib/open_food_network/permalink_generator.rb diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index 986dc07d22..63eb63f0b3 100644 --- a/app/models/enterprise_group.rb +++ b/app/models/enterprise_group.rb @@ -1,6 +1,8 @@ require 'open_food_network/locking' +require 'open_food_network/permalink_generator' class EnterpriseGroup < ActiveRecord::Base + include PermalinkGenerator acts_as_list has_and_belongs_to_many :enterprises @@ -81,25 +83,10 @@ class EnterpriseGroup < ActiveRecord::Base private - def self.find_available_value(existing, requested) - return requested unless existing.include?(requested) - used_indices = existing.map do |p| - p.slice!(/^#{requested}/) - p.match(/^\d+$/).to_s.to_i - end - options = (1..used_indices.length + 1).to_a - used_indices - requested + options.first.to_s - end - - def find_available_permalink(requested) - existing = self.class.where(id: !id).where("permalink LIKE ?", "#{requested}%").pluck(:permalink) - self.class.find_available_value(existing, requested) - end - def sanitize_permalink if permalink.blank? || permalink_changed? requested = permalink.presence || permalink_was.presence || name.presence || 'group' - self.permalink = find_available_permalink(requested.parameterize) + self.permalink = create_unique_permalink(requested.parameterize) end end end diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 2cf13baf84..53f960ba63 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -1,4 +1,7 @@ +require 'open_food_network/permalink_generator' + Spree::Product.class_eval do + include PermalinkGenerator # We have an after_destroy callback on Spree::ProductOptionType. However, if we # don't specify dependent => destroy on this association, it is not called. See: # https://github.com/rails/rails/issues/7618 @@ -19,6 +22,8 @@ Spree::Product.class_eval do attr_accessible :variant_unit, :variant_unit_scale, :variant_unit_name, :unit_value attr_accessible :inherits_properties, :sku + before_validation :sanitize_permalink + # validates_presence_of :variants, unless: :new_record?, message: "Product must have at least one variant" validates_presence_of :supplier validates :primary_taxon, presence: { message: "^Product Category can't be blank" } @@ -239,4 +244,11 @@ Spree::Product.class_eval do raise end end + + def sanitize_permalink + if permalink.blank? || permalink_changed? + requested = permalink.presence || permalink_was.presence || name.presence || 'product' + self.permalink = create_unique_permalink(requested.parameterize) + end + end end diff --git a/lib/open_food_network/permalink_generator.rb b/lib/open_food_network/permalink_generator.rb new file mode 100644 index 0000000000..99d923fd1e --- /dev/null +++ b/lib/open_food_network/permalink_generator.rb @@ -0,0 +1,22 @@ +module PermalinkGenerator + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + def find_available_value(existing, requested) + return requested unless existing.include?(requested) + used_indices = existing.map do |p| + p.slice!(/^#{requested}/) + p.match(/^\d+$/).to_s.to_i + end + options = (1..used_indices.length + 1).to_a - used_indices + requested + options.first.to_s + end + end + + def create_unique_permalink(requested) + existing = self.class.where('id != ?', id).where("permalink LIKE ?", "#{requested}%").pluck(:permalink) + self.class.find_available_value(existing, requested) + end +end