Merge pull request #578 from BBC-News/chromification

Chromification

lib/wraith/compare_images.rb

@@ -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(:*)

lib/wraith/helpers/save_metadata.rb

@@ -1,4 +1,5 @@
 require "wraith"
+require "fileutils"
 
 class SaveMetadata
   attr_reader :wraith, :history
@@ -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
 

lib/wraith/save_images.rb

@@ -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
@@ -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)
@@ -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
@@ -96,12 +96,16 @@ def get_driver
     case meta.engine
     when "chrome"
       options = Selenium::WebDriver::Chrome::Options.new
-      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
@@ -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
@@ -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
@@ -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)

lib/wraith/wraith.rb

@@ -72,7 +72,7 @@ def directory
   end
 
   def history_dir
-    @config["history_dir"] || false
+    @config.fetch('history_dir', false)
   end
 
   def engine
@@ -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
@@ -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
@@ -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
@@ -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

spec/_helpers.rb

@@ -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)

spec/config_spec.rb

@@ -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)
@@ -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

spec/configs/test_config--casper.yaml

@@ -30,4 +30,4 @@ paths:
 fuzz: '20%'
 
 # Number of threads used when saving images
-save_image_threads: 7
+threads: 7

spec/configs/test_config--phantom.yaml

@@ -35,4 +35,4 @@ fuzz: '20%'
 
 
 # Number of threads used when saving images
-save_image_threads: 7
+#threads: 7

spec/js/global--chrome.js

@@ -1,4 +1,4 @@
-var callback = arguments[arguments.length-1];
-document.body.innerHTML = " ";
-document.body.style['background-color'] = 'red';
-callback();
+(function () {
+    document.body.innerHTML = " ";
+    document.body.style['background-color'] = 'red';
+})();

spec/js/path--chrome.js

@@ -1,4 +1,4 @@
-var callback = arguments[arguments.length-1];
-document.body.innerHTML = " ";
-document.body.style['background-color'] = 'green';
-callback();
+(function () {
+    document.body.innerHTML = " ";
+    document.body.style['background-color'] = 'green';
+})();

spec/resize_reload_spec.rb

@@ -21,6 +21,8 @@
           test: http://www.bbc.com
         paths:
           test: /mypage
+        directory:
+          test
         screen_widths:
           - 320
           - 464
@@ -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

templates/configs/capture.yaml

@@ -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

templates/configs/history.yaml

@@ -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

templates/configs/spider.yaml

@@ -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

wraith.gemspec

@@ -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'