Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NEWS-6530 Chromification #578

Merged
merged 14 commits into from
Aug 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file removed assets/invalid.jpg
Binary file not shown.
Binary file added assets/invalid1.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/invalid2.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion lib/wraith/compare_images.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def percentage(img_size, px_value, info)
end

def compare_task(base, compare, output, info)
cmdline = "compare -dissimilarity-threshold 1 -fuzz #{wraith.fuzz} -metric AE -highlight-color #{wraith.highlight_color} #{base} #{compare.shellescape} #{output}"
cmdline = "compare -fuzz #{wraith.fuzz} -metric AE -highlight-color #{wraith.highlight_color} #{base} #{compare.shellescape} #{output}"
px_value = Open3.popen3(cmdline) { |_stdin, _stdout, stderr, _wait_thr| stderr.read }.to_f
begin
img_size = ImageSize.path(output).size.inject(:*)
Expand Down
3 changes: 3 additions & 0 deletions lib/wraith/helpers/save_metadata.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "wraith"
require "fileutils"

class SaveMetadata
attr_reader :wraith, :history
Expand All @@ -14,6 +15,8 @@ def history_label

def file_names(width, label, domain_label)
width = "MULTI" if width.is_a? Array

FileUtils::mkdir_p "#{wraith.directory}/#{label}" # ensure the directory exists
"#{wraith.directory}/#{label}/#{width}_#{engine}_#{domain_label}.png"
end

Expand Down
59 changes: 36 additions & 23 deletions lib/wraith/save_images.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ def define_individual_job(label, settings, width)
compare_file_name = meta.file_names(width, label, meta.compare_label)

jobs = []
jobs << [label, settings.path, prepare_widths_for_cli(width), settings.base_url, base_file_name, settings.selector, wraith.before_capture, settings.before_capture]
jobs << [label, settings.path, prepare_widths_for_cli(width), settings.compare_url, compare_file_name, settings.selector, wraith.before_capture, settings.before_capture] unless settings.compare_url.nil?
jobs << [label, settings.path, prepare_widths_for_cli(width), settings.base_url, base_file_name, settings.selector, wraith.before_capture, settings.before_capture, 'invalid1.jpg']
jobs << [label, settings.path, prepare_widths_for_cli(width), settings.compare_url, compare_file_name, settings.selector, wraith.before_capture, settings.before_capture, 'invalid2.jpg'] unless settings.compare_url.nil?

jobs
end
Expand All @@ -76,7 +76,7 @@ def run_command(command)
end

def parallel_task(jobs)
Parallel.each(jobs, :in_threads => wraith.save_image_threads) do |_label, _path, width, url, filename, selector, global_before_capture, path_before_capture|
Parallel.each(jobs, :in_threads => wraith.threads) do |_label, _path, width, url, filename, selector, global_before_capture, path_before_capture|
begin
if meta.engine == "chrome"
capture_image_selenium(width, url, filename, selector, global_before_capture, path_before_capture)
Expand All @@ -85,8 +85,8 @@ def parallel_task(jobs)
attempt_image_capture(command, filename)
end
rescue => e
logger.error e
create_invalid_image(filename, width)
logger.error "#{e}\n URL = #{url}"
create_invalid_image(filename, width, invalid_image_name)
end
end
end
Expand All @@ -96,12 +96,16 @@ def get_driver
case meta.engine
when "chrome"
options = Selenium::WebDriver::Chrome::Options.new
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we tidy this up? A pattern like [opt1, opt2, ...].each |opt| options.add_argument(opt)

options.add_argument('--disable-gpu')
options.add_argument('--headless')
options.add_argument('--device-scale-factor=1') # have to change cropping for 2x. also this is faster
options.add_argument('--force-device-scale-factor')
options.add_argument("--window-size=1200,1500") # resize later so we can reuse drivers
options.add_argument("--hide-scrollbars") # hide scrollbars from screenshots
[
'disable-gpu',
'headless',
'no-sandbox',
'device-scale-factor=1',
'force-device-scale-factor',
'window-size=1200,1500',
'hide-scrollbars',
'ignore-certificate-errors'
].each { |arg| options.add_argument("--#{arg}") }
Selenium::WebDriver.for :chrome, options: options
end
end
Expand All @@ -111,7 +115,7 @@ def resize_to_fit_page driver
width = driver.execute_script("return Math.max(document.body.scrollWidth, document.body.offsetWidth, document.documentElement.clientWidth, document.documentElement.scrollWidth, document.documentElement.offsetWidth);")
height = driver.execute_script("return Math.max(document.body.scrollHeight, document.body.offsetHeight, document.documentElement.clientHeight, document.documentElement.scrollHeight, document.documentElement.offsetHeight);")
driver.manage.window.resize_to(width, height)
end
end

# crop an image around the coordinates of an element
def crop_selector driver, selector, image_location
Expand All @@ -123,16 +127,25 @@ def crop_selector driver, selector, image_location

def capture_image_selenium(screen_sizes, url, file_name, selector, global_before_capture, path_before_capture)
driver = get_driver
driver.manage.timeouts.implicit_wait = 10;
screen_sizes.to_s.split(",").each do |screen_size|
width, height = screen_size.split("x")
new_file_name = file_name.sub('MULTI', screen_size)
driver.manage.window.resize_to(width, height || 1500)
driver.navigate.to url
driver.execute_async_script(File.read(global_before_capture)) if global_before_capture
driver.execute_async_script(File.read(path_before_capture)) if path_before_capture
resize_to_fit_page(driver) unless height
driver.save_screenshot(new_file_name)
crop_selector(driver, selector, new_file_name) if selector && selector.length > 0
for attempt in 1..3 do
begin
width, height = screen_size.split("x")
new_file_name = file_name.sub('MULTI', screen_size)
driver.manage.window.resize_to(width, height || 1500)
driver.navigate.to url
driver.manage.timeouts.implicit_wait = wraith.settle
driver.execute_script(File.read(global_before_capture)) if global_before_capture
driver.execute_script(File.read(path_before_capture)) if path_before_capture
resize_to_fit_page(driver) unless height
driver.save_screenshot(new_file_name)
crop_selector(driver, selector, new_file_name) if selector && selector.length > 0
break
rescue Net::ReadTimeout => e
logger.error "Got #{e} on attempt #{attempt} at screen size #{screensize}. URL = #{url}"
end
end
end
driver.quit
end
Expand Down Expand Up @@ -164,9 +177,9 @@ def image_was_created(filename)
wraith.resize or File.exist? filename
end

def create_invalid_image(filename, width)
def create_invalid_image(filename, width, invalid_image_name)
logger.warn "Using fallback image instead"
invalid = File.expand_path("../../assets/invalid.jpg", File.dirname(__FILE__))
invalid = File.expand_path("../../assets/#{invalid_image_name}", File.dirname(__FILE__))
FileUtils.cp invalid, filename

set_image_width(filename, width)
Expand Down
47 changes: 18 additions & 29 deletions lib/wraith/wraith.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def directory
end

def history_dir
@config["history_dir"] || false
@config.fetch('history_dir', false)
end

def engine
Expand Down Expand Up @@ -109,7 +109,7 @@ def widths

def resize
# @TODO make this default to true, once it's been tested a bit more thoroughly
@config["resize_or_reload"] ? (@config["resize_or_reload"] == "resize") : false
@config.fetch('resize_or_reload', 'reload') == "resize"
end

def domains
Expand All @@ -132,8 +132,16 @@ def comp_domain_label
domains.keys[1]
end

def settle
@config.fetch('settle', 10)
end

def threads
@config.fetch('threads', '8').to_i
end

def spider_file
@config["spider_file"] ? @config["spider_file"] : "spider.txt"
@config.fetch('spider_file', 'spider.txt')
end

def spider_days
Expand All @@ -157,7 +165,7 @@ def fuzz
end

def highlight_color
@config["highlight_color"] ? @config["highlight_color"] : "blue"
@config.fetch('highlight_color', 'blue')
end

def mode
Expand All @@ -169,50 +177,31 @@ def mode
end

def threshold
@config["threshold"] ? @config["threshold"] : 0
@config.fetch('threshold', 0)
end

def gallery_template
default = "basic_template"
if @config["gallery"].nil?
default
else
@config["gallery"]["template"] || default
end
@config.fetch('gallery', {}).fetch('template', 'basic_template')
end

def thumb_height
default = 200
if @config["gallery"].nil?
default
else
@config["gallery"]["thumb_height"] || default
end
@config.fetch('gallery', {}).fetch('thumb_height', 200)
end

def thumb_width
default = 200
if @config["gallery"].nil?
default
else
@config["gallery"]["thumb_width"] || default
end
@config.fetch('gallery', {}).fetch('thumb_width', 200)
end

def phantomjs_options
@config["phantomjs_options"]
end

def imports
@config['imports'] || false
@config.fetch('imports', false)
end

def verbose
# @TODO - also add a `--verbose` CLI flag which overrides whatever you have set in the config
@config["verbose"] || false
end

def save_image_threads
(@config["save_image_threads"] || 8).to_i
@config.fetch('verbose', false)
end
end
1 change: 0 additions & 1 deletion spec/_helpers.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
require "rspec"
require "./lib/wraith/cli"
require "pry"

def create_diff_image
capture_image = saving.construct_command(320, test_url1, test_image1, selector, false, false)
Expand Down
24 changes: 20 additions & 4 deletions spec/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,26 @@
expect(wraith.config).to include "directory" => "shots"
end

it 'returns default values for threads' do
expect(wraith.threads).to eq 8
end
it 'returns default values for settle' do
expect(wraith.settle).to eq 10
end

context 'non-standard config values' do
let(:config) { YAML.load "browser: phantomjs\nthreads: 2\nsettle: 5"}
let(:non_standard_wraith) { Wraith::Wraith.new( config, { yaml_passed: true }) }

it 'returns overridden value when threads is specified in config' do
expect(non_standard_wraith.threads).to eq 2
end

it 'returns overridden value when settle is specified in config' do
expect(non_standard_wraith.settle).to eq 5
end
end

it "should be able to import other configs" do
config_name = get_path_relative_to __FILE__, "./configs/test_config--imports.yaml"
wraith = Wraith::Wraith.new(config_name)
Expand Down Expand Up @@ -76,10 +96,6 @@
it "include compare label" do
expect(wraith.paths).to eq("home" => "/", "uk_index" => "/uk")
end

it "include save image threads" do
expect(wraith.save_image_threads).to eq(7)
end
end

describe "different ways of initialising browser engine" do
Expand Down
2 changes: 1 addition & 1 deletion spec/configs/test_config--casper.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@ paths:
fuzz: '20%'

# Number of threads used when saving images
save_image_threads: 7
threads: 7
2 changes: 1 addition & 1 deletion spec/configs/test_config--phantom.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ fuzz: '20%'


# Number of threads used when saving images
save_image_threads: 7
#threads: 7
8 changes: 4 additions & 4 deletions spec/js/global--chrome.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var callback = arguments[arguments.length-1];
document.body.innerHTML = "&nbsp;";
document.body.style['background-color'] = 'red';
callback();
(function () {
document.body.innerHTML = "&nbsp;";
document.body.style['background-color'] = 'red';
})();
8 changes: 4 additions & 4 deletions spec/js/path--chrome.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var callback = arguments[arguments.length-1];
document.body.innerHTML = "&nbsp;";
document.body.style['background-color'] = 'green';
callback();
(function () {
document.body.innerHTML = "&nbsp;";
document.body.style['background-color'] = 'green';
})();
6 changes: 4 additions & 2 deletions spec/resize_reload_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
test: http://www.bbc.com
paths:
test: /mypage
directory:
test
screen_widths:
- 320
- 464
Expand All @@ -41,10 +43,10 @@
expect(efficient_jobs.length).to be 1
expect(inefficient_jobs.length).to be 3 # 1 for each screen width

# [["test", "/mypage", "320,464,624", "http://www.bbc.com/mypage", "/test/MULTI__test.png", " ", "false", "false"]]
# [["test", "/mypage", "320,464,624", "http://www.bbc.com/mypage", "test/MULTI__test.png", " ", "false", "false"]]
expect(efficient_jobs[0][2]).to eq "320,464,624"

# [["test", "/mypage", 320, "http://www.bbc.com/mypage", "/test/320__test.png", " ", "false", "false"], ["test", "/mypage", 464, "http://www.bbc.com/mypage", "/test/464__test.png", " ", "false", "false"], ["test", "/mypage", 624, "http://www.bbc.com/mypage", "/test/624__test.png", " ", "false", "false"]]
# [["test", "/mypage", 320, "http://www.bbc.com/mypage", "test/320__test.png", " ", "false", "false"], ["test", "/mypage", 464, "http://www.bbc.com/mypage", "/test/464__test.png", " ", "false", "false"], ["test", "/mypage", 624, "http://www.bbc.com/mypage", "/test/624__test.png", " ", "false", "false"]]
expect(inefficient_jobs[0][2]).to eq 320
end
end
Expand Down
2 changes: 1 addition & 1 deletion templates/configs/capture.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,4 @@ mode: diffs_first

# (optional) Set the number of threads to use when saving images. Raising this value can improve performance, but very high
# values can lead to server connection issues. Set to around 1.5 the available CPU cores for best performance. Default: 8
save_image_threads: 8
threads: 8
2 changes: 1 addition & 1 deletion templates/configs/history.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,4 @@ phantomjs_options: ''

# (optional) Set the number of threads to use when saving images. Raising this value can improve performance, but very high
# values can lead to server connection issues. Set to around 1.5 the available CPU cores for best performance. Default: 8
save_image_threads: 8
threads: 8
2 changes: 1 addition & 1 deletion templates/configs/spider.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,4 @@ mode: diffs_first

# (optional) Set the number of threads to use when saving images. Raising this value can improve performance, but very high
# values can lead to server connection issues. Set to around 1.5 the available CPU cores for best performance. Default: 8
save_image_threads: 8
threads: 8
5 changes: 2 additions & 3 deletions wraith.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,16 @@ Gem::Specification.new do |spec|
spec.version = Wraith::VERSION
spec.authors = ['Dave Blooman', 'Simon Thulbourn', 'Chris Ashton']
spec.email = ['david.blooman@gmail.com', 'simon+github@thulbourn.com', 'chrisashtonweb@gmail.com']
spec.summary = 'Wraith is a screenshot comparison tool, created by developers at BBC News.'
spec.summary = 'Wraith screenshot comparison tool'
spec.description = 'Wraith is a screenshot comparison tool, created by developers at BBC News.'
spec.homepage = 'https://github.com/BBC-News/wraith'
spec.license = 'Apache 2'
spec.license = 'Apache-2.0'

spec.files = `git ls-files`.split($INPUT_RECORD_SEPARATOR)
spec.executables = spec.files.grep(%r{^bin/}) { |f| File.basename(f) }
spec.test_files = spec.files.grep(%r{^(test|spec|features)/})
spec.require_paths = ['lib']

spec.add_development_dependency 'pry'
spec.add_development_dependency 'rspec'
spec.add_development_dependency 'casperjs'

Expand Down