diff --git a/.rspec b/.rspec index 0d858d3a3..c17865513 100644 --- a/.rspec +++ b/.rspec @@ -1,3 +1,5 @@ ---colour --drb --profile +--color +--require rspec/pride +--format PrideFormatter diff --git a/CHANGELOG.md b/CHANGELOG.md index f64fa544b..1658279e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). It uses [CalVer](https://calver.org/) as of May 2019. +## Unreleased +### Changed +* Sped up test suite (~25%) +* Upgrade simple_form (4.1 -> 5; thanks dependabot) + ## [20.01b](https://github.com/berkmancenter/lumendatabase/releases/tag/2020.01b) - 2020-01-31 ### Changed * CMS URL promoted to the top level, so that the CMS is now serving blog and static page content diff --git a/Gemfile b/Gemfile index 12839e44d..81ed46933 100644 --- a/Gemfile +++ b/Gemfile @@ -57,11 +57,11 @@ group :development do gem 'bullet' gem 'derailed' gem 'memory_profiler' - gem 'web-console' end group :development, :test do gem 'factory_bot_rails' + gem 'parallel_tests' gem 'phantomjs' gem 'pry', '~> 0.10.4' gem 'pry-byebug' @@ -85,12 +85,13 @@ group :test do gem 'curb' gem 'database_cleaner' gem 'elasticsearch-extensions' - gem 'fakeweb' gem 'poltergeist' gem 'rack-test', require: 'rack/test' gem 'rails-controller-testing' + gem 'rspec-pride' gem 'shoulda-matchers', '~> 3.1', '>= 3.1.1' gem 'simplecov', require: false + gem 'test-prof' gem 'timecop' gem 'vcr' gem 'webmock' diff --git a/Gemfile.lock b/Gemfile.lock index 280e3c302..3c5ed0ff0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,25 +7,25 @@ GIT GEM remote: https://rubygems.org/ specs: - actioncable (5.2.4.1) - actionpack (= 5.2.4.1) + actioncable (5.2.4.2) + actionpack (= 5.2.4.2) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailer (5.2.4.1) - actionpack (= 5.2.4.1) - actionview (= 5.2.4.1) - activejob (= 5.2.4.1) + actionmailer (5.2.4.2) + actionpack (= 5.2.4.2) + actionview (= 5.2.4.2) + activejob (= 5.2.4.2) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (5.2.4.1) - actionview (= 5.2.4.1) - activesupport (= 5.2.4.1) + actionpack (5.2.4.2) + actionview (= 5.2.4.2) + activesupport (= 5.2.4.2) rack (~> 2.0, >= 2.0.8) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.2.4.1) - activesupport (= 5.2.4.1) + actionview (5.2.4.2) + activesupport (= 5.2.4.2) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) @@ -35,26 +35,26 @@ GEM addressable active_model_serializers (0.8.4) activemodel (>= 3.0) - activejob (5.2.4.1) - activesupport (= 5.2.4.1) + activejob (5.2.4.2) + activesupport (= 5.2.4.2) globalid (>= 0.3.6) - activemodel (5.2.4.1) - activesupport (= 5.2.4.1) + activemodel (5.2.4.2) + activesupport (= 5.2.4.2) activemodel-serializers-xml (1.0.2) activemodel (> 5.x) activesupport (> 5.x) builder (~> 3.1) - activerecord (5.2.4.1) - activemodel (= 5.2.4.1) - activesupport (= 5.2.4.1) + activerecord (5.2.4.2) + activemodel (= 5.2.4.2) + activesupport (= 5.2.4.2) arel (>= 9.0) activerecord-import (1.0.2) activerecord (>= 3.2) - activestorage (5.2.4.1) - actionpack (= 5.2.4.1) - activerecord (= 5.2.4.1) + activestorage (5.2.4.2) + actionpack (= 5.2.4.2) + activerecord (= 5.2.4.2) marcel (~> 0.3.1) - activesupport (5.2.4.1) + activesupport (5.2.4.2) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -69,7 +69,6 @@ GEM arel (9.0.0) bcrypt (3.1.13) benchmark-ips (2.7.2) - bindex (0.8.1) bootsnap (1.4.5) msgpack (~> 1.0) bootstrap-datepicker-rails (1.8.0.1) @@ -114,7 +113,7 @@ GEM sassc-rails (>= 2.0.0) comfy_bootstrap_form (4.0.8) rails (>= 5.0.0) - concurrent-ruby (1.1.5) + concurrent-ruby (1.1.6) country_select (1.2.0) coveralls (0.7.1) multi_json (~> 1.3) @@ -124,7 +123,7 @@ GEM thor crack (0.4.3) safe_yaml (~> 1.0.0) - crass (1.0.5) + crass (1.0.6) curb (0.9.10) database_cleaner (1.7.0) date_validator (0.9.0) @@ -178,7 +177,6 @@ GEM factory_bot_rails (5.0.2) factory_bot (~> 5.0.2) railties (>= 4.2.0) - fakeweb (1.3.0) faraday (0.15.4) multipart-post (>= 1.2, < 3) ffi (1.11.3) @@ -208,7 +206,7 @@ GEM http-accept (1.7.0) http-cookie (1.0.3) domain_name (~> 0.5) - i18n (1.7.0) + i18n (1.8.2) concurrent-ruby (~> 1.0) jquery-placeholder-rails (2.1.2) jquery-rails (4.3.5) @@ -249,11 +247,11 @@ GEM mime-types (3.3) mime-types-data (~> 3.2015) mime-types-data (3.2019.0904) - mimemagic (0.3.3) + mimemagic (0.3.4) mini_magick (4.10.1) mini_mime (1.0.2) mini_portile2 (2.4.0) - minitest (5.13.0) + minitest (5.14.0) msgpack (1.3.1) multi_json (1.13.1) multipart-post (2.1.1) @@ -264,7 +262,7 @@ GEM nested_form (0.3.2) netrc (0.11.0) nio4r (2.5.2) - nokogiri (1.10.7) + nokogiri (1.10.9) mini_portile2 (~> 2.4.0) oink (0.10.1) activerecord @@ -276,6 +274,9 @@ GEM mime-types mimemagic (~> 0.3.0) terrapin (~> 0.6.0) + parallel (1.19.1) + parallel_tests (2.31.0) + parallel pg (1.1.4) phantomjs (2.1.1.0) poltergeist (1.18.1) @@ -291,8 +292,8 @@ GEM pry (~> 0.10) pry-rails (0.3.9) pry (>= 0.10.4) - public_suffix (3.1.1) - rack (2.0.8) + public_suffix (4.0.3) + rack (2.0.9) rack-accept (0.4.5) rack (>= 0.4) rack-attack (6.1.0) @@ -304,18 +305,18 @@ GEM rack (>= 1.1) rack-test (1.1.0) rack (>= 1.0, < 3) - rails (5.2.4.1) - actioncable (= 5.2.4.1) - actionmailer (= 5.2.4.1) - actionpack (= 5.2.4.1) - actionview (= 5.2.4.1) - activejob (= 5.2.4.1) - activemodel (= 5.2.4.1) - activerecord (= 5.2.4.1) - activestorage (= 5.2.4.1) - activesupport (= 5.2.4.1) + rails (5.2.4.2) + actioncable (= 5.2.4.2) + actionmailer (= 5.2.4.2) + actionpack (= 5.2.4.2) + actionview (= 5.2.4.2) + activejob (= 5.2.4.2) + activemodel (= 5.2.4.2) + activerecord (= 5.2.4.2) + activestorage (= 5.2.4.2) + activesupport (= 5.2.4.2) bundler (>= 1.3.0) - railties (= 5.2.4.1) + railties (= 5.2.4.2) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.4) actionpack (>= 5.0.1.x) @@ -341,9 +342,9 @@ GEM rails (>= 5.0, < 7) remotipart (~> 1.3) sassc-rails (>= 1.3, < 3) - railties (5.2.4.1) - actionpack (= 5.2.4.1) - activesupport (= 5.2.4.1) + railties (5.2.4.2) + actionpack (= 5.2.4.2) + activesupport (= 5.2.4.2) method_source rake (>= 0.8.7) thor (>= 0.19.0, < 2.0) @@ -371,6 +372,10 @@ GEM http-cookie (>= 1.0.2, < 2.0) mime-types (>= 1.16, < 4.0) netrc (~> 0.8) + rspec (3.6.0) + rspec-core (~> 3.6.0) + rspec-expectations (~> 3.6.0) + rspec-mocks (~> 3.6.0) rspec-collection_matchers (1.1.3) rspec-expectations (>= 2.99.0.beta1) rspec-core (3.6.0) @@ -381,6 +386,8 @@ GEM rspec-mocks (3.6.0) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.6.0) + rspec-pride (3.2.1) + rspec (~> 3.0) rspec-rails (3.6.0) actionpack (>= 3.0) activesupport (>= 3.0) @@ -414,7 +421,7 @@ GEM rack shoulda-matchers (3.1.3) activesupport (>= 4.0.0) - simple_form (4.1.0) + simple_form (5.0.0) actionpack (>= 5.0) activemodel (>= 5.0) simplecov (0.17.0) @@ -440,6 +447,7 @@ GEM tins (~> 1.0) terrapin (0.6.0) climate_control (>= 0.0.3, < 1.0) + test-prof (0.10.2) therubyracer (0.12.3) libv8 (~> 3.16.14.15) ref @@ -464,15 +472,10 @@ GEM vcr (4.0.0) warden (1.2.7) rack (>= 1.0) - web-console (3.7.0) - actionview (>= 5.0) - activemodel (>= 5.0) - bindex (>= 0.4.0) - railties (>= 5.0) - webmock (3.4.2) + webmock (3.8.0) addressable (>= 2.3.6) crack (>= 0.3.2) - hashdiff + hashdiff (>= 0.4.0, < 2.0.0) websocket-driver (0.7.1) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.4) @@ -507,7 +510,6 @@ DEPENDENCIES elasticsearch-model (~> 5.0) elasticsearch-rails (~> 5.0) factory_bot_rails - fakeweb flutie jquery-placeholder-rails jquery-rails @@ -521,6 +523,7 @@ DEPENDENCIES neat oink paperclip (~> 5) + parallel_tests pg (~> 1.1.4) phantomjs poltergeist @@ -540,6 +543,7 @@ DEPENDENCIES record_tag_helper (~> 1.0) redcarpet rspec-collection_matchers (~> 1.1, >= 1.1.2) + rspec-pride rspec-rails ruby-prof sassc-rails @@ -550,12 +554,12 @@ DEPENDENCIES simplecov skylight stackprof + test-prof therubyracer timecop turnout uglifier vcr - web-console webmock BUNDLED WITH diff --git a/README.md b/README.md index 8d6bd9af8..3f4430fc5 100644 --- a/README.md +++ b/README.md @@ -63,13 +63,27 @@ may use chillingeffects.org rather than lumendatabase.org. #### Running Tests - $ bundle exec rspec spec/ + $ rspec The integration tests are quite slow; for some development purposes you may find it more convenient to `bundle exec rspec spec/ --exclude-pattern="spec/integration/*"`. If `elasticsearch` isn't on your $PATH, set `ENV['TEST_CLUSTER_COMMAND']=/path/to/elasticsearch`, and make sure permissions are set correctly for your test suite to run it. +If you're running a subset of tests that you know don't require Elasticsearch, +you can run them without setting it up via +`TEST_WITH_ELASTICSEARCH=0 rspec path/to/tests`. + +#### Parallelizing Tests +You can speed up tests by running them in parallel: + $ rake parallel:spec + +You will need to do some setup before the first time you run this: +- alter `config/database.yml` so that the test database is `yourproject_test<%= ENV['TEST_ENV_NUMBER'] %>` +- run `rake parallel:setup` + +It will default to using the number of processors parallel_tests believes to be available, but you can change this by setting `ENV['PARALLEL_TEST_PROCESSORS']` to the desired number. + #### Linting Use rubocop and leave the code at least as clean as you found it. If you make diff --git a/app/controllers/notices_controller.rb b/app/controllers/notices_controller.rb index 30319a939..cf52cbda9 100644 --- a/app/controllers/notices_controller.rb +++ b/app/controllers/notices_controller.rb @@ -81,7 +81,7 @@ def json_root_for(klass) end def notice_params - params.require(:notice).except(:type).permit( + raw_params = params.require(:notice).except(:type).permit( :title, :subject, :body, @@ -108,6 +108,7 @@ def notice_params ], works_attributes: work_params ) + raw_params.to_h end def entity_params diff --git a/app/models/copyrighted_url.rb b/app/models/copyrighted_url.rb index 310b5bd36..d230da61c 100644 --- a/app/models/copyrighted_url.rb +++ b/app/models/copyrighted_url.rb @@ -1,9 +1,9 @@ -require 'validates_automatically' require 'default_url_original' +require 'validates_automatically' +require 'validates_urls' class CopyrightedUrl < ApplicationRecord include ValidatesAutomatically include DefaultUrlOriginal - - validates_format_of :url, :url_original, with: /\A([a-z]{3,5}:)?\/\/.+/i + include ValidatesUrls end diff --git a/app/models/infringing_url.rb b/app/models/infringing_url.rb index deaf51427..aecd07af6 100644 --- a/app/models/infringing_url.rb +++ b/app/models/infringing_url.rb @@ -1,12 +1,11 @@ -require 'validates_automatically' require 'default_url_original' +require 'validates_automatically' +require 'validates_urls' class InfringingUrl < ApplicationRecord include ValidatesAutomatically include DefaultUrlOriginal - - validates_format_of :url, :url_original, with: /\A([a-z]{3,5}:)?\/\/.+/i - validates_presence_of :url + include ValidatesUrls def self.get_approximate_count ActiveRecord::Base.connection.execute("SELECT reltuples FROM pg_class WHERE relname = 'infringing_urls'").getvalue(0, 0).to_i diff --git a/app/models/notice_submission_finalizer.rb b/app/models/notice_submission_finalizer.rb index 8744007f4..fc9d64521 100644 --- a/app/models/notice_submission_finalizer.rb +++ b/app/models/notice_submission_finalizer.rb @@ -1,8 +1,12 @@ +require 'param_flattener' + # This performs final updates to a Notice instance that has been created, but # not yet supplied with all its attributes. The goal here is to be able to # move slow parts of the notice creation process outside the request/response # cycle. class NoticeSubmissionFinalizer + include ParamFlattener + delegate :errors, to: :notice def initialize(notice, parameters) @@ -13,11 +17,51 @@ def initialize(notice, parameters) def finalize parameters[:submission_id] = notice.id notice.works.delete(NoticeSubmissionInitializer::PLACEHOLDER_WORKS) + fix_concatenated_urls(:infringing_urls_attributes) + fix_concatenated_urls(:copyrighted_urls_attributes) notice.update_attributes(parameters) end private - attr_reader :parameters + attr_accessor :parameters attr_accessor :notice + + # Sometimes we get submissions which, instead of containing a single URL, + # contain a long list of URLs, concatenated. We don't necessarily persist + # these to the db as they may exceed the 1.kilobyte allowed length. When we + # see this problem, we should instead create a CopyrightedUrl instance for + # each URL, as this is surely the original intent. + def fix_concatenated_urls(attr) + flatten_param(parameters[:works_attributes]).each do |work_hash| + next unless work_hash[attr].present? + + new_hashes = [] + + flatten_param(work_hash[attr]).each do |url_hash| + next unless url_hash[:url].scan('/http').present? + + split_urls = conservative_split(url_hash[:url]) + + # Overwrite the current URL with one of the split-apart URLs. Then + # add the rest of the split-apart URLs to a list for safekeeping. + url_hash[:url] = split_urls.pop() + split_urls.map { |url| new_hashes << { url: url } } + end + + # Now that we know all the URLs which needed to be un-concatenated, + # update the work hash. We're doing that here because we don't want to + # modify it while looping over its contents! + work_hash[attr] += new_hashes.uniq if new_hashes.present? + end + end + + # We can't just split on 'http', because doing so will result in strings + # which no longer contain it. We need to look at the pairs of 'http' and + # $the_rest_of_the_URL which split produces and then mash them back together. + def conservative_split(s) + b = [] + s.split(/(http)/).reject { |x| x.blank? }.each_slice(2) { |s| b << s.join } + b + end end diff --git a/app/models/notice_submission_initializer.rb b/app/models/notice_submission_initializer.rb index d3f98d945..931502f66 100644 --- a/app/models/notice_submission_initializer.rb +++ b/app/models/notice_submission_initializer.rb @@ -1,8 +1,12 @@ +require 'param_flattener' + # This performs initial steps of the Notice submission process: creating a # new Notice with enough of the provided attributes that it can be persisted. # Where persisting attributes is slow, those can be deferred to the # NoticeSubmissionFinalizer. class NoticeSubmissionInitializer + include ParamFlattener + delegate :errors, to: :notice # Notice validates the presence of works, but we delay adding works because @@ -37,7 +41,7 @@ def add_notice_defaults def notice @notice ||= model_class.new( - if parameters.class == Hash + if ([ActiveSupport::HashWithIndifferentAccess, Hash].include? parameters.class) parameters else parameters.permit! @@ -94,23 +98,4 @@ def entity_notice_role(role_name) roles = flatten_param(parameters[:entity_notice_roles_attributes]) roles.detect { |role| role[:name] == role_name } end - - # JSON API submissions: - # - # [{ ... }, { ... }, { ... }] - # - # Rails form submissions: - # - # { '0' => { ... }, '1' => { ... }, '3' => { ... } } - # - # This flattens the second style to the first, IFF it's not that way - # already. Curse you, Rails for making me type-check. - def flatten_param(param) - case param - when ActionController::Parameters then param.values - when Hash then param.values - when Array then param - else [] - end - end end diff --git a/lib/param_flattener.rb b/lib/param_flattener.rb new file mode 100644 index 000000000..9af98728e --- /dev/null +++ b/lib/param_flattener.rb @@ -0,0 +1,22 @@ +module ParamFlattener + private + + # JSON API submissions: + # + # [{ ... }, { ... }, { ... }] + # + # Rails form submissions: + # + # { '0' => { ... }, '1' => { ... }, '3' => { ... } } + # + # This flattens the second style to the first, IFF it's not that way + # already. Curse you, Rails for making me type-check. + def flatten_param(param) + case param + when ActionController::Parameters then param.values + when Hash then param.values + when Array then param + else [] + end + end +end diff --git a/lib/validates_urls.rb b/lib/validates_urls.rb new file mode 100644 index 000000000..1a67d4015 --- /dev/null +++ b/lib/validates_urls.rb @@ -0,0 +1,31 @@ +module ValidatesUrls + def self.included(model) + model.send(:validates, 'url'.freeze, { presence: true } ) + model.send(:validates, 'url_original'.freeze, { presence: true } ) + model.send(:validate, :good_urls?) + end + + private + + def good_urls? + %i[url url_original].each do |attr| + # URI::regexp will fail for things like "//bar.com", but we want to + # allow those. + value = self.send(attr) + next unless value.present? + + unless ( is_ordinary_uri?(value) || is_noprotocol_uri?(value) ) + errors.add(attr, 'Must be a valid URL') + end + end + end + + def is_ordinary_uri?(value) + !!(value =~ URI::regexp) + end + + # Matches things like "//bar.com". + def is_noprotocol_uri?(value) + value.start_with?('//') && (('http:' + value) =~ URI::regexp) + end +end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index f2eba2837..99ee4c598 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -4,7 +4,7 @@ include ApplicationHelper context '#can_see_full_notice_version?' do - let(:notice) { create(:dmca) } + let(:notice) { build(:dmca) } let(:token_url) do create( :token_url, diff --git a/spec/helpers/notices_helper_spec.rb b/spec/helpers/notices_helper_spec.rb index 1a8f36f7d..1852609c5 100644 --- a/spec/helpers/notices_helper_spec.rb +++ b/spec/helpers/notices_helper_spec.rb @@ -6,78 +6,78 @@ it 'returns correct form partials' do # Not testing every one of these in the interests of test speed, but # checking both a one-word and a multi-word notice type. - expect(helper.form_partial_for(create(:dmca))).to eq 'dmca_form' - expect(helper.form_partial_for(create(:law_enforcement_request))) + expect(helper.form_partial_for(build(:dmca))).to eq 'dmca_form' + expect(helper.form_partial_for(build(:law_enforcement_request))) .to eq 'law_enforcement_request_form' end it 'returns correct show partials' do - expect(helper.show_partial_for(create(:dmca))).to eq 'dmca_show' - expect(helper.show_partial_for(create(:law_enforcement_request))) + expect(helper.show_partial_for(build(:dmca))).to eq 'dmca_show' + expect(helper.show_partial_for(build(:law_enforcement_request))) .to eq 'law_enforcement_request_show' end it 'returns correct works partials' do - expect(helper.works_partial_for(create(:dmca))).to eq 'dmca_works' - expect(helper.works_partial_for(create(:law_enforcement_request))) + expect(helper.works_partial_for(build(:dmca))).to eq 'dmca_works' + expect(helper.works_partial_for(build(:law_enforcement_request))) .to eq 'law_enforcement_request_works' end it 'displays date_received correctly' do now = Time.utc(2018, 12, 21, 0, 0, 0) - dmca = create(:dmca, date_received: now) + dmca = build(:dmca, date_received: now) expect(helper.date_received(dmca)) .to eq '' end it 'handles missing date_received' do - dmca = create(:dmca, date_received: nil) + dmca = build(:dmca, date_received: nil) expect(helper.date_received(dmca)).to be nil end it 'displays date_sent correctly' do now = Time.utc(2018, 12, 21, 0, 0, 0) - dmca = create(:dmca, date_sent: now) + dmca = build(:dmca, date_sent: now) expect(helper.date_sent(dmca)) .to eq '' end it 'handles missing date_sent' do - dmca = create(:dmca, date_sent: nil) + dmca = build(:dmca, date_sent: nil) expect(helper.date_sent(dmca)).to be nil end it 'displays notice subjects' do subject = 'Cockapoos vs corgis: go' - dmca = create(:dmca, subject: subject) + dmca = build(:dmca, subject: subject) expect(helper.subject(dmca)).to eq subject end it 'handles missing notice subjects' do - dmca = create(:dmca, subject: nil) + dmca = build(:dmca, subject: nil) expect(helper.subject(dmca)).to eq 'Unknown' end it 'displays notice sent_via' do source = 'Internet megacorp' - dmca = create(:dmca, source: source) + dmca = build(:dmca, source: source) expect(helper.sent_via(dmca)).to eq source end it 'handles missing notice sent_via' do - dmca = create(:dmca, source: nil) + dmca = build(:dmca, source: nil) expect(helper.sent_via(dmca)).to eq 'Unknown' end it 'correctly labels URL inputs' do - court_order = create(:court_order) - data_protection = create(:data_protection) - defamation = create(:defamation) - dmca = create(:dmca) - law_enforcement = create(:law_enforcement_request) - other = create(:other) - private_information = create(:private_information) - trademark = create(:trademark) + court_order = build(:court_order) + data_protection = build(:data_protection) + defamation = build(:defamation) + dmca = build(:dmca) + law_enforcement = build(:law_enforcement_request) + other = build(:other) + private_information = build(:private_information) + trademark = build(:trademark) expect(helper.label_for_url_input(:infringing_urls, court_order)) .to eq 'Targeted URL' @@ -105,14 +105,14 @@ end it 'handles bogus URL types' do - dmca = create(:dmca) + dmca = build(:dmca) expect { helper.label_for_url_input(:party_time_urls, dmca) } .to raise_error(RuntimeError) end context 'permanent_url_full_notice' do - let(:notice) { create(:dmca) } - let(:user) { create(:user) } + let(:notice) { build(:dmca) } + let(:user) { build(:user) } it 'returns false when no token url is found' do allow_any_instance_of(NoticesHelper).to receive(:current_user).and_return(user) @@ -123,7 +123,7 @@ end it 'returns a proper url when a token url is found' do - token_url = create_token_url + token_url = build_token_url allow_any_instance_of(NoticesHelper).to receive(:current_user).and_return(user) @@ -137,7 +137,7 @@ end it 'returns false when current_user is nil' do - create_token_url + build_token_url allow_any_instance_of(NoticesHelper).to receive(:current_user).and_return(nil) @@ -149,7 +149,7 @@ private - def create_token_url + def build_token_url create( :token_url, notice: notice, diff --git a/spec/integration/blog_feed_spec.rb b/spec/integration/blog_feed_spec.rb deleted file mode 100644 index fac4cae78..000000000 --- a/spec/integration/blog_feed_spec.rb +++ /dev/null @@ -1,54 +0,0 @@ -require 'rails_helper' -require 'rss' - -feature 'blog rss feed' do - include ComfyHelpers - include Comfy::CmsHelper - include ERB::Util - - before :all do - Rake::Task['lumen:set_up_cms'].execute - @site = Comfy::Cms::Site.find_by_identifier('lumen_cms') - @layout = Comfy::Cms::Layout.find_by_label('blawg') - @blog = Comfy::Cms::Page.find_by_label('blog_entries') - - 15.times do |i| - BlogPostFactory.new(@site, @layout, @blog, seed: i).manufacture - end - end - - after :all do - destroy_cms - end - - it 'resolves at the expected URL' do - visit '/blog_feed.rss' - expect(page).to have_http_status(200) - end - - it 'contains the latest content' do - visit '/blog_feed.rss' - - @blog.children.last(10).each do |post| - expect(page).to have_content cms_fragment_content('title', post) - expect(page).to have_content cms_fragment_content('author', post) - expect(page).to have_content post.created_at.to_s(:rfc822) - expect(page).to have_content post.url - expect(page).to have_content cms_fragment_content('content', post) - expect(page).to have_content cms_fragment_content('abstract', post) - end - end - - # Not attempting to validate it against a schema here; just smoke-testing - # that it is parseable as RSS. If it isn't, the parser will raise an - # exception. - it 'is probably valid' do - visit '/blog_feed.rss' - RSS::Parser.parse(page.body) - end - - it 'is available from the home page' do - visit '/' - expect(page).to have_link(href: 'blog_feed.rss') - end -end diff --git a/spec/integration/cms_blog_entries_spec.rb b/spec/integration/cms_spec.rb similarity index 79% rename from spec/integration/cms_blog_entries_spec.rb rename to spec/integration/cms_spec.rb index fb93ede45..093e7d818 100644 --- a/spec/integration/cms_blog_entries_spec.rb +++ b/spec/integration/cms_spec.rb @@ -1,11 +1,15 @@ -require 'rails_helper' require 'comfy/blog_post_factory' +require 'rails_helper' +require 'rss' feature 'CMS blog entries' do include ComfyHelpers include Comfy::ComfyHelper include Comfy::CmsHelper + include ERB::Util + # Try to put all CMS integration specs in this file, so that we only need to + # do this costly setup once. before :all do Rake::Task['lumen:set_up_cms'].execute @site = Comfy::Cms::Site.find_by_identifier('lumen_cms') @@ -14,7 +18,7 @@ end after :each do - Comfy::Cms::Page.where.not(id: @blog.id).delete_all + @blog.children.destroy_all @blog.reload end @@ -31,11 +35,11 @@ visit @blog.full_path (0..9).each do |i| - expect(page).to have_link("page_#{i}") + expect(page).to have_link("page_#{i}", exact: true) end (10..14).each do |i| - expect(page).not_to have_link("page_#{i}") + expect(page).not_to have_link("page_#{i}", exact: true) end visit "#{@blog.full_path}?page=2" @@ -47,7 +51,7 @@ end (10..14).each do |i| - expect(page).to have_link("page_#{i}") + expect(page).to have_link("page_#{i}", exact: true) end end @@ -59,8 +63,8 @@ visit @blog.full_path - expect(page).to have_link('page_0') - expect(page).not_to have_link('page_1') + expect(page).to have_link('page_0', exact: true) + expect(page).not_to have_link('page_1', exact: true) end it 'displays working links' do @@ -76,6 +80,7 @@ 2.times do |i| BlogPostFactory.new(@site, @layout, @blog, seed: i).manufacture end + @blog.reload # Switch around the expected creation dates, to make sure we're sorting # on created_at and not id. @blog.children.first.update(created_at: 2.days.ago) @@ -227,6 +232,49 @@ end end + context 'feed' do + before :all do + 15.times do |i| + BlogPostFactory.new(@site, @layout, @blog, seed: i).manufacture + end + end + + after :all do + @blog.children.delete_all + end + + it 'resolves at the expected URL' do + visit '/blog_feed.rss' + expect(page).to have_http_status(200) + end + + it 'contains the latest content' do + visit '/blog_feed.rss' + + @blog.children.last(10).each do |post| + expect(page).to have_content cms_fragment_content('title', post) + expect(page).to have_content cms_fragment_content('author', post) + expect(page).to have_content post.created_at.to_s(:rfc822) + expect(page).to have_content post.url + expect(page).to have_content cms_fragment_content('content', post) + expect(page).to have_content cms_fragment_content('abstract', post) + end + end + + # Not attempting to validate it against a schema here; just smoke-testing + # that it is parseable as RSS. If it isn't, the parser will raise an + # exception. + it 'is probably valid' do + visit '/blog_feed.rss' + RSS::Parser.parse(page.body) + end + + it 'is available from the home page' do + visit '/' + expect(page).to have_link(href: 'blog_feed.rss') + end + end + def have_custom_search_engine_embed have_css('.blog-search') end diff --git a/spec/integration/fielded_notice_search_spec.rb b/spec/integration/fielded_notice_search_spec.rb index 166ab7930..4cabbf681 100644 --- a/spec/integration/fielded_notice_search_spec.rb +++ b/spec/integration/fielded_notice_search_spec.rb @@ -150,9 +150,7 @@ notice = create(:dmca, :with_facet_data, title: 'Lion King two') index_changed_instances - search_on_page = FieldedSearchOnPage.new - search_on_page.visit_search_page(true) - search_on_page.open_advanced_search + search_on_page = init_search_on_page search_on_page.add_fielded_search_for(title_field, 'lion') @@ -177,28 +175,20 @@ end context 'active' do - before :each do - search_on_page = FieldedSearchOnPage.new - search_on_page.visit_search_page - search_on_page.open_advanced_search - end - scenario 'dropdown retains visibility between page views', search: true, js: true do - search_on_page = FieldedSearchOnPage.new - search_on_page.open_advanced_search + skip 'This fails intermittently due to the time it takes to set cookies' + search_on_page = init_search_on_page expect(page).to have_visible_advanced_search_controls - sleep 0.2 - search_on_page.visit_search_page expect(page).to have_visible_advanced_search_controls end scenario 'retains query parameters', search: true, js: true do - search_on_page = FieldedSearchOnPage.new + search_on_page = init_search_on_page search_on_page.add_fielded_search_for(title_field, 'lion') - search_on_page.run_search(false) + search_on_page.run_search search_on_page.open_advanced_search search_on_page.within_fielded_searches do @@ -211,7 +201,7 @@ # attack = "'/>
0 click_on 'Attach another' - sleep 0.2 end field_name = "notice_file_uploads_attributes_#{@field_index}_file" diff --git a/spec/support/page_objects/fielded_search_on_page.rb b/spec/support/page_objects/fielded_search_on_page.rb index bf1f8e4ac..bb734ebb7 100644 --- a/spec/support/page_objects/fielded_search_on_page.rb +++ b/spec/support/page_objects/fielded_search_on_page.rb @@ -2,9 +2,8 @@ class FieldedSearchOnPage < PageObject def open_advanced_search - return if advanced_search_visible? - sleep ENV['SEARCH_SLEEP'].to_i if ENV['SEARCH_SLEEP'] - find('a#toggle-advanced-search').click + node = first('.container.advanced-search', visible: :all) + find('a#toggle-advanced-search').click unless node.visible? end def add_more @@ -47,8 +46,8 @@ def define_sort_order(sort_order) end def open_sort_order_menu - return if sort_order_visible? - find('.sort-order a.dropdown-toggle').click + menu = first('.sort-order ol.dropdown-menu', visible: :all) + find('.sort-order a.dropdown-toggle').click unless menu.visible? end def change_field(from, to) @@ -65,39 +64,19 @@ def remove_fielded_search_for(field) end end - def run_search(wait_for_index = true) - if wait_for_index - sleep((ENV['SEARCH_SLEEP'] && ENV['SEARCH_SLEEP'].to_i) || 1) - end - + def run_search find('.advanced-search .resubmit .button').click end - def visit_search_page(wait_for_index = false) - if wait_for_index - sleep((ENV['SEARCH_SLEEP'] && ENV['SEARCH_SLEEP'].to_i) || 1) - end - + def visit_search_page visit '/notices/search' end def parameterized_search_for(field, term) - sleep((ENV['SEARCH_SLEEP'] && ENV['SEARCH_SLEEP'].to_i) || 1) - visit "/notices/search?#{field}=#{CGI.escape(term)}" end def within_results(&block) within('.results-list', &block) end - - private - - def advanced_search_visible? - first('.container.advanced-search', minimum: 0) - end - - def sort_order_visible? - first('.sort-order ol.dropdown-menu', minimum: 0) - end end diff --git a/spec/support/search_helpers.rb b/spec/support/search_helpers.rb index e62202ece..eb0f293b2 100644 --- a/spec/support/search_helpers.rb +++ b/spec/support/search_helpers.rb @@ -8,8 +8,6 @@ def index_changed_instances end def submit_search(term) - sleep (ENV["SEARCH_SLEEP"] && ENV["SEARCH_SLEEP"].to_i) || 1 # required for indexing to complete - visit '/' fill_in 'search', with: term @@ -17,8 +15,6 @@ def submit_search(term) end def search_for(searches) - sleep (ENV["SEARCH_SLEEP"] && ENV["SEARCH_SLEEP"].to_i) || 1 # required for indexing to complete - query = searches.map { |k,v| "#{k}=#{CGI.escape(v.to_s)}" }.join('&') visit "/notices/search?#{query}" @@ -47,8 +43,6 @@ def open_and_select_facet(facet, facet_value) end def submit_faceted_search(term, facet, facet_value) - sleep (ENV["SEARCH_SLEEP"] && ENV["SEARCH_SLEEP"].to_i) || 1 # required for indexing to complete - visit '/' fill_in 'search', with: term diff --git a/spec/support/webmock.rb b/spec/support/webmock.rb index 6b1a59c37..0a387655b 100644 --- a/spec/support/webmock.rb +++ b/spec/support/webmock.rb @@ -1,3 +1,6 @@ require 'webmock/rspec' -WebMock.allow_net_connect! +# Don't make calls to populate the Twitter widget during tests. +# (More generally, don't fail tests based on the availability of external +# services, and don't make a ton of external calls during tests.) +WebMock.disable_net_connect! allow_localhost: true diff --git a/spec/tasks/generate_court_order_report_spec.rb b/spec/tasks/generate_court_order_report_spec.rb index b0e8d495d..258c351c6 100644 --- a/spec/tasks/generate_court_order_report_spec.rb +++ b/spec/tasks/generate_court_order_report_spec.rb @@ -12,7 +12,9 @@ after :all do ENV['USER_CRON_EMAIL'] = @cached_user_cron_email - CourtOrder.destroy_all + [CourtOrder, Work, Entity].each do |model| + model.destroy_all + end end it 'sends a single email' do