From 7392aaf52d33165eb71a99b3a4a12ec386f8e936 Mon Sep 17 00:00:00 2001 From: Henric Trotzig Date: Fri, 27 Nov 2015 21:17:56 +0100 Subject: [PATCH 1/4] Store results of diffux run in result_summary.yaml This is a pre-requisite for implementing uploading baselines for new examples, as described in #63. We currently don't store anything differently in the file system when an examples is new, or when there's no diff. To be able to upload new baselines after a run, we need somewhere to store that distinction. I considered the following alternatives: A) Storing a meta tag on the PNG image with information about if it is new or not. B) Storing an extra file inside the example's folder, with information about if it's new or not. C) Storing a summary log from a diffux run. A and B both have N+1 performance implications, so I decided to go with C, which could potentially be used for other things as well. --- lib/diffux_ci_runner.rb | 26 ++++++++++++++++++++++++++ spec/diffux_ci_spec.rb | 41 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/lib/diffux_ci_runner.rb b/lib/diffux_ci_runner.rb index 477ed0e..20846ec 100644 --- a/lib/diffux_ci_runner.rb +++ b/lib/diffux_ci_runner.rb @@ -8,6 +8,7 @@ require 'oily_png' require 'diffux_ci_utils' require 'fileutils' +require 'yaml' def resolve_viewports(example) configured_viewports = DiffuxCIUtils.config['viewports'] @@ -50,6 +51,13 @@ def init_driver fail "JavaScript errors found during initialization: \n#{errors.inspect}" end + # Initialize a hash to store a summary of the results from the run + result_summary = { + new_examples: [], + diff_examples: [], + okay_examples: [] + } + all_examples = driver.execute_script('return window.diffux.getAllExamples()') # To avoid the overhead of resizing the window all the time, we are going to @@ -169,10 +177,18 @@ def init_driver print '.' puts " #{comparison[:diff_in_percent].round(1)}% (#{candidate_path})" + result_summary[:diff_examples] << { + description: description, + viewport: viewport['name'] + } else # No visual difference was found, so we don't need to do any more # work. puts ' No diff.' + result_summary[:okay_examples] << { + description: description, + viewport: viewport['name'] + } end else # There was no baseline image yet, so we want to start by saving a new @@ -185,9 +201,19 @@ def init_driver screenshot.save(baseline_path, :fast_rgba) print '.' puts " First snapshot created (#{baseline_path})" + result_summary[:new_examples] << { + description: description, + viewport: viewport['name'] + } end end end + + result_summary_file = File.join(DiffuxCIUtils.config['snapshots_folder'], + 'result_summary.yaml') + File.open(result_summary_file, 'w') do |file| + file.write result_summary.to_yaml + end ensure driver.quit end diff --git a/spec/diffux_ci_spec.rb b/spec/diffux_ci_spec.rb index 943e856..8a532f4 100644 --- a/spec/diffux_ci_spec.rb +++ b/spec/diffux_ci_spec.rb @@ -63,7 +63,7 @@ def snapshot_file_exists?(description, size, file_name) expect(run_diffux[:exit_status]).to eq(0) end - it 'generates a baseline, but no diff' do + it 'generates a new baseline, but no diff' do run_diffux expect(snapshot_file_exists?(description, '@large', 'baseline.png')) .to eq(true) @@ -71,6 +71,19 @@ def snapshot_file_exists?(description, size, file_name) .to eq(false) expect(snapshot_file_exists?(description, '@large', 'candidate.png')) .to eq(false) + expect( + YAML.load(File.read(File.join( + @tmp_dir, 'snapshots', 'result_summary.yaml'))) + ).to eq( + new_examples: [ + { + description: description, + viewport: 'large' + } + ], + diff_examples: [], + okay_examples: [] + ) end end @@ -92,6 +105,19 @@ def snapshot_file_exists?(description, size, file_name) .to eq(false) expect(snapshot_file_exists?(description, '@large', 'candidate.png')) .to eq(false) + expect( + YAML.load(File.read(File.join( + @tmp_dir, 'snapshots', 'result_summary.yaml'))) + ).to eq( + okay_examples: [ + { + description: description, + viewport: 'large' + } + ], + new_examples: [], + diff_examples: [] + ) end end @@ -124,6 +150,19 @@ def snapshot_file_exists?(description, size, file_name) .to eq(true) expect(snapshot_file_exists?(description, '@large', 'candidate.png')) .to eq(true) + expect( + YAML.load(File.read(File.join( + @tmp_dir, 'snapshots', 'result_summary.yaml'))) + ).to eq( + diff_examples: [ + { + description: description, + viewport: 'large' + } + ], + new_examples: [], + okay_examples: [] + ) end end end From 416e3c73f30039ba2f44d6ad473a732b56cc89a3 Mon Sep 17 00:00:00 2001 From: Henric Trotzig Date: Fri, 27 Nov 2015 21:57:20 +0100 Subject: [PATCH 2/4] Strip newlines from base64 encoded descriptions For some reason, Base64.encode64 returns strings with a newline at the end. This has been causing a few issues: - Logs show awkward newlines - Folder names look weird I'm fixing that by stripping out the newline. --- lib/diffux_ci_utils.rb | 2 +- spec/diffux_ci_spec.rb | 2 +- spec/diffux_ci_utils_spec.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/diffux_ci_utils.rb b/lib/diffux_ci_utils.rb index 5b67fe4..08b79ba 100644 --- a/lib/diffux_ci_utils.rb +++ b/lib/diffux_ci_utils.rb @@ -34,7 +34,7 @@ def self.config_from_file end def self.normalize_description(description) - Base64.encode64(description) + Base64.encode64(description).strip end def self.path_to(description, viewport_name, file_name) diff --git a/spec/diffux_ci_spec.rb b/spec/diffux_ci_spec.rb index 8a532f4..aef71de 100644 --- a/spec/diffux_ci_spec.rb +++ b/spec/diffux_ci_spec.rb @@ -54,7 +54,7 @@ def run_diffux def snapshot_file_exists?(description, size, file_name) File.exist?( File.join(@tmp_dir, 'snapshots', - Base64.encode64(description), size, file_name) + Base64.encode64(description).strip, size, file_name) ) end diff --git a/spec/diffux_ci_utils_spec.rb b/spec/diffux_ci_utils_spec.rb index fed8a8e..b94fb79 100644 --- a/spec/diffux_ci_utils_spec.rb +++ b/spec/diffux_ci_utils_spec.rb @@ -48,7 +48,7 @@ context 'with special characters' do let(:description) { ' something interesting' } - it { should eq(Base64.encode64(description)) } + it { should eq(Base64.encode64(description).strip) } end end @@ -57,6 +57,6 @@ let(:description) { '' } let(:viewport_name) { 'large' } let(:file_name) { 'diff.png' } - it { should eq("./snapshots/#{Base64.encode64(description)}/@large/diff.png") } + it { should eq("./snapshots/#{Base64.encode64(description).strip}/@large/diff.png") } end end From 8b532043d1ff67fca6e208c21f50ebbfdea02d3e Mon Sep 17 00:00:00 2001 From: Henric Trotzig Date: Fri, 27 Nov 2015 22:03:24 +0100 Subject: [PATCH 3/4] Upload new baselines in `diffux upload_diffs` When you add new diffux examples, it is helpful to have those included as "diffs" when reviewing. This commit makes use of the result_summary.yaml file that I just added, and uploads all new snapshots as well. Fixes #63. --- lib/diffux_ci-diffs.html.erb | 11 +++++++++++ lib/diffux_ci_uploader.rb | 20 ++++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/diffux_ci-diffs.html.erb b/lib/diffux_ci-diffs.html.erb index 8a61337..0637bda 100644 --- a/lib/diffux_ci-diffs.html.erb +++ b/lib/diffux_ci-diffs.html.erb @@ -8,11 +8,22 @@

Diffux-CI diffs

File generated: <%= Time.now %>

+

Diffs

<% diff_images.each do |diff| %>

<%= Rack::Utils.escape_html(diff[:description]) %> @ <%= diff[:viewport] %>

<% end %> + +
+ +

New examples

+ <% new_images.each do |image| %> +

+ <%= Rack::Utils.escape_html(image[:description]) %> @ <%= image[:viewport] %> +

+

+ <% end %> diff --git a/lib/diffux_ci_uploader.rb b/lib/diffux_ci_uploader.rb index f6ff337..0ac4fce 100644 --- a/lib/diffux_ci_uploader.rb +++ b/lib/diffux_ci_uploader.rb @@ -17,16 +17,32 @@ def upload_diffs dir = SecureRandom.uuid - diff_images = current_snapshots[:diffs].map do |diff| + result_summary = YAML.load(File.read(File.join( + DiffuxCIUtils.config['snapshots_folder'], 'result_summary.yaml'))) + diff_images = result_summary[:diff_examples].map do |diff| image = bucket.objects.build( "#{dir}/#{diff[:description]}_#{diff[:viewport]}.png") - image.content = open(diff[:file]) + image.content = open(DiffuxCIUtils.path_to(diff[:description], + diff[:viewport], + 'diff.png')) image.content_type = 'image/png' image.save diff[:url] = image.url diff end + new_images = result_summary[:new_examples].map do |example| + image = bucket.objects.build( + "#{dir}/#{example[:description]}_#{example[:viewport]}.png") + image.content = open(DiffuxCIUtils.path_to(example[:description], + example[:viewport], + 'baseline.png')) + image.content_type = 'image/png' + image.save + example[:url] = image.url + example + end + html = bucket.objects.build("#{dir}/index.html") path = File.expand_path( File.join(File.dirname(__FILE__), 'diffux_ci-diffs.html.erb')) From 774edd1095362f1a478f60d5958956a49e792dab Mon Sep 17 00:00:00 2001 From: Henric Trotzig Date: Fri, 27 Nov 2015 22:05:32 +0100 Subject: [PATCH 4/4] Add s3 credentials to example project .diffux_ci.yaml Not actual credentials, but references to environment variables. This makes testing uploading diffs easier for me. --- example-project/.diffux_ci.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/example-project/.diffux_ci.yaml b/example-project/.diffux_ci.yaml index 5988c2b..6ee20ee 100644 --- a/example-project/.diffux_ci.yaml +++ b/example-project/.diffux_ci.yaml @@ -11,3 +11,6 @@ viewports: # http://stackoverflow.com/questions/12463643/mobile-safari-on-iphone-5-visible-area-size width: 320 height: 444 +s3_access_key_id: <%= ENV['S3_ACCESS_KEY_ID'] %> +s3_secret_access_key: <%= ENV['S3_SECRET_ACCESS_KEY'] %> +s3_bucket_name: <%= ENV['S3_BUCKET_NAME'] %>