Skip to content

Commit

Permalink
Improvements to shared test suite.
Browse files Browse the repository at this point in the history
- Use VCR to capture HTTP interactions for s3 integration tests.
- Use Timecop instead of sleep.
- Clean up shared-test boilerplate.
- Move AWS resource#to_a call outside of multithread block (attempt to resolve `JMESPath::Runtime#search` bug potentially caused by thread contention)
- replace Fakeweb with WebMock
  • Loading branch information
wjordan committed Jan 18, 2016
1 parent 67bbadd commit 328e804
Show file tree
Hide file tree
Showing 47 changed files with 6,339 additions and 159 deletions.
5 changes: 4 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ group :development, :test do
gem 'active_record_query_trace'
# for unit testing
gem 'factory_girl_rails'
gem 'fakeweb'
gem 'webmock', require: false
gem 'vcr', require: false

gem 'simplecov', '~> 0.9', require: false
gem 'mocha', require: false
gem "codeclimate-test-reporter", require: false
Expand All @@ -64,6 +66,7 @@ group :development, :test do
gem 'spring-commands-testunit'
gem "minitest", "~> 5.5"
gem 'minitest-reporters'
gem 'minitest-around'
gem 'eyes_selenium', '~> 2.5.0'
end

Expand Down
19 changes: 15 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ GEM
railties (>= 3.0, < 5)
coderay (1.1.0)
colorize (0.7.7)
crack (0.4.2)
safe_yaml (~> 1.0.0)
cucumber (2.0.2)
builder (>= 2.1.2)
cucumber-core (~> 1.2.0)
Expand Down Expand Up @@ -151,7 +153,6 @@ GEM
fake_sqs (0.3.1)
builder
sinatra
fakeweb (1.3.0)
faraday (0.9.1)
multipart-post (>= 1.2, < 3)
ffi (1.9.10)
Expand Down Expand Up @@ -189,6 +190,7 @@ GEM
haml (~> 4.0)
rubocop (>= 0.25.0)
sysexits (~> 1.1)
hashdiff (0.2.3)
hashie (3.4.2)
heroku_rails_deflate (1.0.3)
actionpack (>= 3.2.13)
Expand All @@ -213,8 +215,7 @@ GEM
jbuilder (1.5.3)
activesupport (>= 3.0.0)
multi_json (>= 1.2.0)
jmespath (1.0.2)
multi_json (~> 1.0)
jmespath (1.1.3)
jquery-cookie-rails (1.3.1.1)
railties (>= 3.2.0, < 5.0)
jquery-rails (3.1.4)
Expand Down Expand Up @@ -257,6 +258,8 @@ GEM
mini_magick (4.3.3)
mini_portile (0.6.2)
minitest (5.8.2)
minitest-around (0.3.2)
minitest (~> 5.0)
minitest-reporters (1.0.19)
ansi
builder
Expand Down Expand Up @@ -400,6 +403,7 @@ GEM
ruby_parser (3.7.0)
sexp_processor (~> 4.1)
rubyzip (1.1.7)
safe_yaml (1.0.4)
sass (3.2.19)
sass-rails (4.0.5)
railties (>= 4.0.0, < 5.0)
Expand Down Expand Up @@ -478,13 +482,18 @@ GEM
rack
raindrops (~> 0.7)
user_agent_parser (2.1.5)
vcr (3.0.0)
warden (1.2.3)
rack (>= 1.0)
web-console (2.2.1)
activemodel (>= 4.0)
binding_of_caller (>= 0.7.2)
railties (>= 4.0)
sprockets-rails (>= 2.0, < 4.0)
webmock (1.22.3)
addressable (>= 2.3.6)
crack (>= 0.3.2)
hashdiff
websocket (1.0.7)
websocket-driver (0.6.2)
websocket-extensions (>= 0.1.0)
Expand Down Expand Up @@ -521,7 +530,6 @@ DEPENDENCIES
eyes_selenium (~> 2.5.0)
factory_girl_rails
fake_sqs
fakeweb
font-awesome-rails
geocoder
google-api-client
Expand All @@ -545,6 +553,7 @@ DEPENDENCIES
marked-rails
mini_magick
minitest (~> 5.5)
minitest-around
minitest-reporters
mocha
mysql2 (~> 0.3.13)
Expand Down Expand Up @@ -598,7 +607,9 @@ DEPENDENCIES
uglifier (>= 1.3.0)
unicorn (~> 4.8.2)
user_agent_parser
vcr
web-console (~> 2.0)
webmock
youtube-dl.rb

BUNDLED WITH
Expand Down
4 changes: 2 additions & 2 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,10 @@ task :build => ['build:all']
##
##################################################################################################

# Whether this is a development or adhoc environment where we should install npm and create
# Whether this is a local or adhoc environment where we should install npm and create
# a local database.
def local_environment?
(rack_env?(:development) && !CDO.chef_managed) || rack_env?(:adhoc)
(rack_env?(:development, :test) && !CDO.chef_managed) || rack_env?(:adhoc)
end

def install_npm
Expand Down
5 changes: 3 additions & 2 deletions circle.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
machine:
ruby:
version: 2.2.3
environment:
RAILS_ENV: test
services:
- docker
dependencies:
Expand All @@ -9,9 +11,8 @@ dependencies:
- "~/.rvm/gems/ruby-2.2.3"
override:
- sudo apt-get install libicu-dev
- bundle install
- "printf \"use_dynamo_tables: false\nbundler_use_sudo: false\n\" > locals.yml"
- rake install:pegasus
- "printf \"assets_s3_directory: assets_circle/$RANDOM\nsources_s3_directory: sources_circle/$RANDOM\n\" > locals.yml"
- npm install:
pwd:
apps
Expand Down
38 changes: 19 additions & 19 deletions dashboard/test/controllers/media_proxy_controller_test.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
require 'fakeweb'
require 'webmock/minitest'
WebMock.disable_net_connect!(:allow_localhost => true)
require 'test_helper'

class MediaProxyControllerTest < ActionController::TestCase

IMAGE_URI = 'http://www.example.com/foo.jpg'
IMAGE_DATA = 'JPG_\u0000\u00FF'.force_encoding(Encoding::BINARY)

Expand All @@ -12,7 +12,7 @@ class MediaProxyControllerTest < ActionController::TestCase
'audio/ogg', 'audio/vnd.wav']

content_types.each do |content_type|
FakeWeb.register_uri(:get, IMAGE_URI, body: IMAGE_DATA, content_type: content_type)
stub_request(:get, IMAGE_URI).to_return(body: IMAGE_DATA, headers: {content_type: content_type})
get :get, u: IMAGE_URI
assert_response :success
assert_equal content_type, response.content_type
Expand All @@ -27,49 +27,49 @@ class MediaProxyControllerTest < ActionController::TestCase

test "should fetch proxied media with redirects" do
# Cycle through a redirect followed by the actual data.
FakeWeb.register_uri(:get, IMAGE_URI, [
{body: 'Redirect', status: 302, location: IMAGE_URI},
{body: IMAGE_DATA, content_type: 'image/jpeg'}])

stub_request(:get, IMAGE_URI).to_return(
{body: 'Redirect', status: 302, headers: {location: IMAGE_URI}},
{body: IMAGE_DATA, headers: {content_type: 'image/jpeg'}}
)
get :get, u: IMAGE_URI
assert_response :success
assert_equal IMAGE_DATA, response.body
end

test "should fail if invalid URL" do
FakeWeb.register_uri(:get, IMAGE_URI, body: IMAGE_DATA, content_type: 'image/jpeg')
stub_request(:get, IMAGE_URI).to_return(body: IMAGE_DATA, headers: {content_type: 'image/jpeg'})
bad_uri = '/foo'
get :get, u: bad_uri
assert_response 400
end

test "should fail if missing URL" do
FakeWeb.register_uri(:get, IMAGE_URI, body: IMAGE_DATA, content_type: 'image/jpeg')
stub_request(:get, IMAGE_URI).to_return(body: IMAGE_DATA, headers: {content_type: 'image/jpeg'})
get :get, foo: 'bar'
assert_response 400
end

test "should fail if too many redirects" do
FakeWeb.register_uri(:get, IMAGE_URI, [
{body: 'Redirect', status: 302, location: IMAGE_URI},
{body: 'Redirect', status: 302, location: IMAGE_URI},
{body: 'Redirect', status: 302, location: IMAGE_URI},
{body: 'Redirect', status: 302, location: IMAGE_URI},
{body: 'Redirect', status: 302, location: IMAGE_URI},
{body: 'Redirect', status: 302, location: IMAGE_URI}])

response = {body: 'Redirect', status: 302, headers: {location: IMAGE_URI}}
stub_request(:get, IMAGE_URI).to_return(
response,
response,
response,
response,
response,
response)
get :get, u: IMAGE_URI
assert_response 500
end

test "should fail on unauthorized content types" do
FakeWeb.register_uri(:get, IMAGE_URI, body: IMAGE_DATA, content_type: 'text/html')
stub_request(:get, IMAGE_URI).to_return(body: IMAGE_DATA, headers: {content_type: 'text/html'})
get :get, u: IMAGE_URI
assert_response 400
end

test "should pass through server errors" do
FakeWeb.register_uri(:get, IMAGE_URI, body: IMAGE_DATA, content_type: 'image/jpeg', status: 503)
stub_request(:get, IMAGE_URI).to_return(body: IMAGE_DATA, headers: {content_type: 'image/jpeg'}, status: 503)
get :get, u: IMAGE_URI
assert_response 503
end
Expand Down
3 changes: 0 additions & 3 deletions dashboard/test/lib/queue_processor_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ def reset_received_bodies
end

def setup
# We need to allow http connections from the test to either the real or FAKE SQS service.
FakeWeb.allow_net_connect = true

@sqs = Aws::SQS::Client.new
unless ENV['USE_REAL_SQS'] == 'true'
$fake_sqs_service.start
Expand Down
5 changes: 3 additions & 2 deletions deployment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,9 @@ def rack_env()
CDO.rack_env
end

def rack_env?(env)
rack_env == env
def rack_env?(*env)
e = *env
e.include? rack_env
end

def deploy_dir(*dirs)
Expand Down
4 changes: 2 additions & 2 deletions lib/cdo/analytics/milestone_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ def count
match && !(IGNORE_HOSTS.include? match[:host])
end
logs = Parallel.map(hosts, in_threads: 16) do |host|
s3_resource.bucket('cdo-logs').objects(prefix: "#{host}dashboard/milestone.log").to_a
end.flatten
s3_resource.bucket('cdo-logs').objects(prefix: "#{host}dashboard/milestone.log")
end.map(&:to_a).flatten
debug "Found #{logs.length} logs.."
counts = logs.map do |log|
(cache[log.key] = count_lines_of_code(log))['count']
Expand Down
7 changes: 6 additions & 1 deletion lib/cdo/aws/s3.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,16 @@ def self.download_from_bucket(bucket, key, options={})
# @return [String] The key of the new value, derived from filename.
def self.upload_to_bucket(bucket, filename, data, options={})
no_random = options.delete(:no_random)
filename = "#{SecureRandom.hex}-#{filename}" unless no_random
filename = "#{random}-#{filename}" unless no_random
create_client.put_object(options.merge(bucket: bucket, key: filename, body: data))
filename
end

# Allow the RNG to be stubbed in tests
def self.random
SecureRandom.hex
end

def self.public_url(bucket, filename)
Aws::S3::Object.new(bucket, filename, region: CDO.aws_region).public_url
end
Expand Down
3 changes: 2 additions & 1 deletion shared/middleware/files_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ def record_event(quota_event_type, quota_type, encrypted_channel_id)
not_modified if result[:status] == 'NOT_MODIFIED'
last_modified result[:last_modified]

abuse_score = result[:metadata]['abuse_score'].to_i
metadata = result[:metadata]
abuse_score = [metadata['abuse_score'].to_i, metadata['abuse-score'].to_i].max

This comment has been minimized.

Copy link
@Bjvanminnen

Bjvanminnen Jan 20, 2016

Contributor

do you know why we have both/if we could avoid that?

This comment has been minimized.

Copy link
@islemaster

islemaster Mar 13, 2018

Contributor

@wjordan I know this is old - @epeach and I are running into this right now while trying to write new tests over this API. We're seeing what looks like inconsistent responses from the S3 ruby SDK in how metadata is keyed. Do you remember if that's consistent with your experience here? Did you ever find any resources for this?

This comment has been minimized.

Copy link
@wjordan

wjordan Mar 13, 2018

Author Contributor

@islemaster I don't think I had traced it down fully at the time of this commit, but it looks like webmock replaces underscores with dashes in HTTP headers it handles: bblimke/webmock#474

not_found if abuse_score > 0 and !can_view_abusive_assets?(encrypted_channel_id)

result[:body]
Expand Down
9 changes: 7 additions & 2 deletions shared/middleware/helpers/bucket_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,13 @@ def get(encrypted_channel_id, filename, if_modified_since = nil, version = nil)
end

def get_abuse_score(encrypted_channel_id, filename, version = nil)
response = get(encrypted_channel_id, filename, version)
response.nil? ? 0 : response[:metadata]['abuse_score'].to_i
response = get(encrypted_channel_id, filename, nil, version)
if response.nil?
0
else
metadata = response[:metadata]
[metadata['abuse_score'].to_i, metadata['abuse-score'].to_i].max
end
end

def copy_files(src_channel, dest_channel)
Expand Down

0 comments on commit 328e804

Please sign in to comment.