From 645a55f19b71bb951f3ce7c3a7915ce878f82e97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 10 Mar 2017 03:28:20 +0100 Subject: [PATCH 1/4] Introduce a new middleware for the test environment that can block requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The idea is that after each feature spec example, we block all incoming requests at the Rack level, go to the 'about:blank' page, and wait until the current requests reach 0. This should solve the problem where a request would end after database cleaner performed the database truncation. The problem was that a GET request can still lead to records creation (e.g. namespaces or routes). Signed-off-by: Rémy Coutable --- Gemfile | 1 + Gemfile.lock | 3 +- config/environments/test.rb | 3 + .../testing/request_blocker_middleware.rb | 61 +++++++++++++++++++ spec/spec_helper.rb | 1 + spec/support/wait_for_requests.rb | 40 ++++++++++++ 6 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 lib/gitlab/testing/request_blocker_middleware.rb create mode 100644 spec/support/wait_for_requests.rb diff --git a/Gemfile b/Gemfile index 6af27ce0f3ef..381583876423 100644 --- a/Gemfile +++ b/Gemfile @@ -327,6 +327,7 @@ group :test do gem 'test_after_commit', '~> 1.1' gem 'sham_rack', '~> 1.3.6' gem 'timecop', '~> 0.8.0' + gem 'concurrent-ruby', '~> 1.0.5' end gem 'octokit', '~> 4.6.2' diff --git a/Gemfile.lock b/Gemfile.lock index 043ca4f8800c..07be5d7aded6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -128,7 +128,7 @@ GEM execjs coffee-script-source (1.10.0) colorize (0.7.7) - concurrent-ruby (1.0.4) + concurrent-ruby (1.0.5) connection_pool (2.2.1) crack (0.4.3) safe_yaml (~> 1.0.0) @@ -868,6 +868,7 @@ DEPENDENCIES chronic (~> 0.10.2) chronic_duration (~> 0.10.6) coffee-rails (~> 4.1.0) + concurrent-ruby (~> 1.0.5) connection_pool (~> 2.0) creole (~> 0.5.0) d3_rails (~> 3.5.0) diff --git a/config/environments/test.rb b/config/environments/test.rb index fb25d3a8b14c..a25c5016a3b5 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -1,4 +1,7 @@ Rails.application.configure do + # Make sure the middleware is inserted first in middleware chain + config.middleware.insert_before('ActionDispatch::Static', 'Gitlab::Testing::RequestBlockerMiddleware') + # Settings specified here will take precedence over those in config/application.rb # The test environment is used exclusively to run your application's diff --git a/lib/gitlab/testing/request_blocker_middleware.rb b/lib/gitlab/testing/request_blocker_middleware.rb new file mode 100644 index 000000000000..aa67fa08577a --- /dev/null +++ b/lib/gitlab/testing/request_blocker_middleware.rb @@ -0,0 +1,61 @@ +# rubocop:disable Style/ClassVars + +# This is inspired by http://www.salsify.com/blog/engineering/tearing-capybara-ajax-tests +# Rack middleware that keeps track of the number of active requests and can block new requests. +module Gitlab + module Testing + class RequestBlockerMiddleware + @@num_active_requests = Concurrent::AtomicFixnum.new(0) + @@block_requests = Concurrent::AtomicBoolean.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 + + private + + def block_requests? + @@block_requests.true? + end + + def block_request(env) + [503, {}, []] + end + + def increment_active_requests + @@num_active_requests.increment + end + + def decrement_active_requests + @@num_active_requests.decrement + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ceb3209331fb..ad2ecd75d744 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -35,6 +35,7 @@ config.include Warden::Test::Helpers, type: :request config.include LoginHelpers, type: :feature config.include SearchHelpers, type: :feature + config.include WaitForRequests, :js config.include WaitForAjax, type: :feature config.include StubConfiguration config.include EmailHelpers, type: :mailer diff --git a/spec/support/wait_for_requests.rb b/spec/support/wait_for_requests.rb new file mode 100644 index 000000000000..4a8958c73360 --- /dev/null +++ b/spec/support/wait_for_requests.rb @@ -0,0 +1,40 @@ +module WaitForRequests + extend self + + # This is inspired by http://www.salsify.com/blog/engineering/tearing-capybara-ajax-tests + def wait_for_requests_complete + stop_client + Gitlab::Testing::RequestBlockerMiddleware.block_requests! + wait_for('pending AJAX requests complete') do + Gitlab::Testing::RequestBlockerMiddleware.num_active_requests.zero? + end + ensure + Gitlab::Testing::RequestBlockerMiddleware.allow_requests! + end + + # Navigate away from the current page which will prevent any new requests from being started + def stop_client + page.execute_script %Q{ + window.location = "about:blank"; + } + end + + # Waits until the passed block returns true + def wait_for(condition_name, max_wait_time: Capybara.default_max_wait_time, polling_interval: 0.01) + wait_until = Time.now + max_wait_time.seconds + loop do + break if yield + if Time.now > wait_until + raise "Condition not met: #{condition_name}" + else + sleep(polling_interval) + end + end + end +end + +RSpec.configure do |config| + config.after(:each, :js) do + wait_for_requests_complete + end +end -- GitLab From 7322770dda0f94885790324d1fc653e20491e7bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 22 Mar 2017 19:05:16 +0100 Subject: [PATCH 2/4] Only include WaitForAjax for :js specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/spec_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ad2ecd75d744..5ab8f0d981a0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -36,7 +36,7 @@ config.include LoginHelpers, type: :feature config.include SearchHelpers, type: :feature config.include WaitForRequests, :js - config.include WaitForAjax, type: :feature + config.include WaitForAjax, :js config.include StubConfiguration config.include EmailHelpers, type: :mailer config.include TestEnv -- GitLab From f879e40da73b0c149c0928883ad8965224fab8d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 22 Mar 2017 19:40:35 +0100 Subject: [PATCH 3/4] Don't navigate to about:blank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This turned out to prevent the browser from clearing the local storage. Signed-off-by: Rémy Coutable --- spec/support/wait_for_requests.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/spec/support/wait_for_requests.rb b/spec/support/wait_for_requests.rb index 4a8958c73360..0bfa7f72ff8c 100644 --- a/spec/support/wait_for_requests.rb +++ b/spec/support/wait_for_requests.rb @@ -3,7 +3,6 @@ module WaitForRequests # This is inspired by http://www.salsify.com/blog/engineering/tearing-capybara-ajax-tests def wait_for_requests_complete - stop_client Gitlab::Testing::RequestBlockerMiddleware.block_requests! wait_for('pending AJAX requests complete') do Gitlab::Testing::RequestBlockerMiddleware.num_active_requests.zero? @@ -12,13 +11,6 @@ def wait_for_requests_complete Gitlab::Testing::RequestBlockerMiddleware.allow_requests! end - # Navigate away from the current page which will prevent any new requests from being started - def stop_client - page.execute_script %Q{ - window.location = "about:blank"; - } - end - # Waits until the passed block returns true def wait_for(condition_name, max_wait_time: Capybara.default_max_wait_time, polling_interval: 0.01) wait_until = Time.now + max_wait_time.seconds -- GitLab From 709a566700df5cb2b43adfd1ad90a997aa36251d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 22 Mar 2017 19:04:29 +0100 Subject: [PATCH 4/4] Fix spec/features/participants_autocomplete_spec.rb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .../participants_autocomplete_spec.rb | 95 ++++++------------- 1 file changed, 28 insertions(+), 67 deletions(-) diff --git a/spec/features/participants_autocomplete_spec.rb b/spec/features/participants_autocomplete_spec.rb index c2545b0c2591..decad589c23d 100644 --- a/spec/features/participants_autocomplete_spec.rb +++ b/spec/features/participants_autocomplete_spec.rb @@ -1,101 +1,62 @@ require 'spec_helper' -feature 'Member autocomplete', feature: true do - include WaitForAjax - +feature 'Member autocomplete', :js do let(:project) { create(:project, :public) } let(:user) { create(:user) } - let(:participant) { create(:user) } let(:author) { create(:user) } + let(:note) { create(:note, noteable: noteable, project: noteable.project) } before do - allow_any_instance_of(Commit).to receive(:author).and_return(author) - login_as user + note # actually create the note + login_as(user) end - shared_examples "open suggestions" do - it 'displays suggestions' do - expect(page).to have_selector('.atwho-view', visible: true) - end - - it 'suggests author' do - page.within('.atwho-view', visible: true) do - expect(page).to have_content(author.username) + shared_examples "open suggestions when typing @" do + before do + page.within('.new-note') do + find('#note_note').send_keys('@') end end - it 'suggests participant' do + it 'suggests noteable author and note author' do page.within('.atwho-view', visible: true) do - expect(page).to have_content(participant.username) + expect(page).to have_content(author.username) + expect(page).to have_content(note.author.username) end end end - context 'adding a new note on a Issue', js: true do + context 'adding a new note on a Issue' do + let(:noteable) { create(:issue, author: author, project: project) } before do - issue = create(:issue, author: author, project: project) - create(:note, note: 'Ultralight Beam', noteable: issue, - project: project, author: participant) - visit_issue(project, issue) + visit namespace_project_issue_path(project.namespace, project, noteable) end - context 'when typing @' do - include_examples "open suggestions" - before do - open_member_suggestions - end - end + include_examples "open suggestions when typing @" end - context 'adding a new note on a Merge Request ', js: true do + context 'adding a new note on a Merge Request' do + let(:noteable) do + create(:merge_request, source_project: project, + target_project: project, author: author) + end before do - merge = create(:merge_request, source_project: project, target_project: project, author: author) - create(:note, note: 'Ultralight Beam', noteable: merge, - project: project, author: participant) - visit_merge_request(project, merge) + visit namespace_project_merge_request_path(project.namespace, project, noteable) end - context 'when typing @' do - include_examples "open suggestions" - before do - open_member_suggestions - end - end + include_examples "open suggestions when typing @" end - context 'adding a new note on a Commit ', js: true do - let(:commit) { project.commit } + context 'adding a new note on a Commit' do + let(:noteable) { project.commit } + let(:note) { create(:note_on_commit, project: project, commit_id: project.commit.id) } before do - allow(commit).to receive(:author).and_return(author) - create(:note_on_commit, author: participant, project: project, commit_id: project.repository.commit.id, note: 'No More Parties in LA') - visit_commit(project, commit) - end - - context 'when typing @' do - include_examples "open suggestions" - before do - open_member_suggestions - end - end - end + allow_any_instance_of(Commit).to receive(:author).and_return(author) - def open_member_suggestions - page.within('.new-note') do - find('#note_note').send_keys('@') + visit namespace_project_commit_path(project.namespace, project, noteable) end - wait_for_ajax - end - - def visit_issue(project, issue) - visit namespace_project_issue_path(project.namespace, project, issue) - end - - def visit_merge_request(project, merge) - visit namespace_project_merge_request_path(project.namespace, project, merge) - end - def visit_commit(project, commit) - visit namespace_project_commit_path(project.namespace, project, commit) + include_examples "open suggestions when typing @" end end -- GitLab