Dduvall has uploaded a new change for review. https://gerrit.wikimedia.org/r/230230
Change subject: Decouple screenshot-ing and artifacts from Cucumber hooks ...................................................................... Decouple screenshot-ing and artifacts from Cucumber hooks Created `ScreenshotHelper` to encapsulate and generalize the screenshot-ing previously implemented directly in a Cucumber `After` hook. Refactored `Environment#teardown` and friends to return artifacts in the form `{ path => mime_type }` which can then be logged in the way appropriate to the test runner. Moved logging of SauceLabs job URL to a `RemoteBrowserFactory#teardown` and refactored `HeadlessHelper` for the new artifacts system. Bug: T108273 Change-Id: I4534fcb05180419ee9aa354f12616aadec160659 --- A features/screenshots.feature A features/step_definitions/screenshot_steps.rb M lib/mediawiki_selenium.rb M lib/mediawiki_selenium/browser_factory/base.rb M lib/mediawiki_selenium/environment.rb M lib/mediawiki_selenium/remote_browser_factory.rb M lib/mediawiki_selenium/support/env.rb M lib/mediawiki_selenium/support/hooks.rb M lib/mediawiki_selenium/support/modules/headless_helper.rb A lib/mediawiki_selenium/support/modules/screenshot_helper.rb A spec/screenshot_helper_spec.rb 11 files changed, 230 insertions(+), 36 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/selenium refs/changes/30/230230/1 diff --git a/features/screenshots.feature b/features/screenshots.feature new file mode 100644 index 0000000..84be3f7 --- /dev/null +++ b/features/screenshots.feature @@ -0,0 +1,21 @@ +@integration +Feature: Screenshots of failed scenarios + + As a developer writing and running tests, it would be helpful to have a + screenshot of the browser window at the point where each scenario has + failed. + + Background: + Given I have configured my environment with: + """ + screenshot_failures: true + screenshot_failures_path: tmp/screenshots + """ + And the "tmp/screenshots" directory exists + And I am using the screenshot helper + + Scenario: A screenshot is taken for failed scenarios + Given the current scenario name is "Some scenario" + And I have started a browser + When the scenario fails + Then the file "tmp/screenshots/Some scenario.png" should exist diff --git a/features/step_definitions/screenshot_steps.rb b/features/step_definitions/screenshot_steps.rb new file mode 100644 index 0000000..52fedbd --- /dev/null +++ b/features/step_definitions/screenshot_steps.rb @@ -0,0 +1,3 @@ +Given(/^I am using the screenshot helper$/) do + @env.extend(MediawikiSelenium::ScreenshotHelper) +end diff --git a/lib/mediawiki_selenium.rb b/lib/mediawiki_selenium.rb index 505c50a..ee3a256 100644 --- a/lib/mediawiki_selenium.rb +++ b/lib/mediawiki_selenium.rb @@ -9,6 +9,7 @@ autoload :PageFactory, 'mediawiki_selenium/page_factory' autoload :Raita, 'mediawiki_selenium/raita' autoload :RemoteBrowserFactory, 'mediawiki_selenium/remote_browser_factory' + autoload :ScreenshotHelper, 'mediawiki_selenium/support/modules/screenshot_helper' autoload :StrictPending, 'mediawiki_selenium/support/modules/strict_pending' autoload :UserFactory, 'mediawiki_selenium/user_factory' autoload :UserFactoryHelper, 'mediawiki_selenium/support/modules/user_factory_helper' diff --git a/lib/mediawiki_selenium/browser_factory/base.rb b/lib/mediawiki_selenium/browser_factory/base.rb index 8684bfd..70a6815 100644 --- a/lib/mediawiki_selenium/browser_factory/base.rb +++ b/lib/mediawiki_selenium/browser_factory/base.rb @@ -200,8 +200,12 @@ # @param _env [Environment] Environment. # @param _status [Symbol] Status of the executed scenario. # + # @return [Hash{String => String}] Artifacts. + # + # @see Environment#teardown + # def teardown(_env, _status) - # abstract + {} end protected diff --git a/lib/mediawiki_selenium/environment.rb b/lib/mediawiki_selenium/environment.rb index f772707..61b9460 100644 --- a/lib/mediawiki_selenium/environment.rb +++ b/lib/mediawiki_selenium/environment.rb @@ -359,9 +359,13 @@ # Executes teardown tasks including instructing all browser factories to # close any open browsers and perform their own teardown tasks. # + # Teardown tasks may produce artifacts, which will be returned in the form + # `{ path => mime_type }`. + # # @example Teardown environment resources after each scenario completes # After do |scenario| - # teardown(name: scenario.name, status: scenario.status) + # artifacts = teardown(name: scenario.name, status: scenario.status) + # artifacts.each { |path, mime_type| embed(path, mime_type) } # end # # @param info [Hash] Hash of test case information. @@ -369,15 +373,22 @@ # @yield [browser] # @yieldparam browser [Watir::Browser] Browser object, before it's closed. # + # @return [Hash{String => String}] Hash of path/mime-type artifacts. + # def teardown(info = {}) + artifacts = {} + @_factory_cache.each do |(_, browser_name), factory| factory.each do |browser| yield browser if block_given? browser.close unless keep_browser_open? && browser_name != :phantomjs end - factory.teardown(self, info[:status] || :passed) + factory_artifacts = factory.teardown(self, info[:status] || :passed) + artifacts.merge!(factory_artifacts) if factory_artifacts.is_a?(Hash) end + + {} end # Returns the current value for `:mediawiki_user` or the value for the diff --git a/lib/mediawiki_selenium/remote_browser_factory.rb b/lib/mediawiki_selenium/remote_browser_factory.rb index 6756891..64bcaf8 100644 --- a/lib/mediawiki_selenium/remote_browser_factory.rb +++ b/lib/mediawiki_selenium/remote_browser_factory.rb @@ -42,6 +42,8 @@ # Submits status and Jenkins build info to Sauce Labs. # def teardown(env, status) + artifacts = super + each do |browser| sid = browser.driver.session_id url = browser.driver.send(:bridge).http.send(:server_url) @@ -62,7 +64,11 @@ ) Cucumber::Formatter::Sauce.current_session_id = sid + + artifacts["http://saucelabs.com/jobs/#{sid}"] = 'text/url' end + + artifacts end protected diff --git a/lib/mediawiki_selenium/support/env.rb b/lib/mediawiki_selenium/support/env.rb index a4d50e4..a6ec7af 100644 --- a/lib/mediawiki_selenium/support/env.rb +++ b/lib/mediawiki_selenium/support/env.rb @@ -6,5 +6,6 @@ World(MediawikiSelenium::ApiHelper) World(MediawikiSelenium::PageFactory) +World(MediawikiSelenium::ScreenshotHelper) World(MediawikiSelenium::StrictPending) World(MediawikiSelenium::UserFactoryHelper) diff --git a/lib/mediawiki_selenium/support/hooks.rb b/lib/mediawiki_selenium/support/hooks.rb index 8618742..70cff16 100644 --- a/lib/mediawiki_selenium/support/hooks.rb +++ b/lib/mediawiki_selenium/support/hooks.rb @@ -56,7 +56,7 @@ end end -Before do |scenario| +Before do # Create a unique random string for this scenario @random_string = Random.new.rand.to_s @@ -75,26 +75,12 @@ end After do |scenario| - if scenario.respond_to?(:status) - require 'fileutils' - - teardown(name: @scenario_name, status: scenario.status) do |browser| - # Embed remote session URLs - if remote? && browser.driver.respond_to?(:session_id) - embed("http://saucelabs.com/jobs/#{browser.driver.session_id}", 'text/url') - end - - # Take screenshots - if scenario.failed? && lookup(:screenshot_failures, default: false) == 'true' - screen_dir = lookup(:screenshot_failures_path, default: 'screenshots') - FileUtils.mkdir_p screen_dir - name = @scenario_name.gsub(/ /, '_') - path = "#{screen_dir}/#{name}.png" - browser.screenshot.save path - embed path, 'image/png' - end + artifacts = + if scenario.respond_to?(:status) + teardown(name: @scenario_name, status: scenario.status) + else + teardown(name: @scenario_name) end - else - teardown(name: @scenario_name) - end + + artifacts.each { |path, mime_type| embed(path, mime_type) } end diff --git a/lib/mediawiki_selenium/support/modules/headless_helper.rb b/lib/mediawiki_selenium/support/modules/headless_helper.rb index 87f0050..0683d9e 100644 --- a/lib/mediawiki_selenium/support/modules/headless_helper.rb +++ b/lib/mediawiki_selenium/support/modules/headless_helper.rb @@ -96,21 +96,29 @@ # @see Environment#teardown # def teardown(info = {}) - super - ensure - if @_headless_capture - if info[:status] == :failed - dir = File.absolute_path(headless_capture_path) - FileUtils.mkdir_p(dir) + artifacts = {} - filename = "#{(info[:name] || 'scenario').tr("#{File::SEPARATOR}\000", '-')}.mp4" - filename = File.join(dir, filename) + begin + artifacts = super + ensure + if @_headless_capture + if info[:status] == :failed + dir = File.absolute_path(headless_capture_path) + FileUtils.mkdir_p(dir) - @_headless_display.video.stop_and_save(filename) - else - @_headless_display.video.stop_and_discard + filename = "#{(info[:name] || 'scenario').tr("#{File::SEPARATOR}\000", '-')}.mp4" + filename = File.join(dir, filename) + + @_headless_display.video.stop_and_save(filename) + + artifacts[filename] = 'video/mp4' + else + @_headless_display.video.stop_and_discard + end end end + + artifacts end end end diff --git a/lib/mediawiki_selenium/support/modules/screenshot_helper.rb b/lib/mediawiki_selenium/support/modules/screenshot_helper.rb new file mode 100644 index 0000000..dd2a5bd --- /dev/null +++ b/lib/mediawiki_selenium/support/modules/screenshot_helper.rb @@ -0,0 +1,58 @@ +require 'fileutils' + +module MediawikiSelenium + # Adds support to {Environment} for taking screenshots of the current + # browser window. If `screenshot_failures` is set to `true` for the current + # environment, one is automatically taken at the end of each failed + # test case. + # + module ScreenshotHelper + # Takes a screenshot of the given browser window and returns the path + # to the saved image. + # + # @param browser [Watir::Browser] Browser to take a sceeenshot of. + # @param name [String] Base name to use for the saved image file. + # + # @return [String] Path to the new screenshot. + # + def screenshot(browser, name) + dir = screenshot_failures_path + FileUtils.mkdir_p dir + + name = name.tr("#{File::SEPARATOR}\000", '-') + + path = File.join(dir, "#{name}.png") + browser.screenshot.save(path) + + path + end + + # Takes screenshots for failed tests. + # + # @param info [Hash] Test case information. + # + def teardown(info = {}) + screenshots = [] + + artifacts = super(info) do |browser| + if info[:status] == :failed && screenshot_failures? + screenshots << screenshot(browser, info[:name] || 'scenario') + end + + yield browser if block_given? + end + + artifacts.merge(screenshots.each.with_object({}) { |img, arts| arts[img] = 'image/png' }) + end + + private + + def screenshot_failures? + lookup(:screenshot_failures, default: false).to_s == 'true' + end + + def screenshot_failures_path + lookup(:screenshot_failures_path, default: 'screenshots') + end + end +end diff --git a/spec/screenshot_helper_spec.rb b/spec/screenshot_helper_spec.rb new file mode 100644 index 0000000..759bd9a --- /dev/null +++ b/spec/screenshot_helper_spec.rb @@ -0,0 +1,95 @@ +require 'spec_helper' + +module MediawikiSelenium + describe ScreenshotHelper do + let(:env) { Environment.new(config).extend(ScreenshotHelper) } + + let(:config) { {} } + + describe '#screenshot' do + subject { env.screenshot(browser, name) } + + let(:config) { { screenshot_failures_path: 'screenshots' } } + + let(:browser) { double('Watir::Browser') } + let(:name) { 'Some test name' } + + it 'takes a screenshot of the browser' do + screenshot = double('Watir::Screenshot') + expect(browser).to receive(:screenshot).and_return(screenshot) + expect(screenshot).to receive(:save).with('screenshots/Some test name.png') + + subject + end + + it 'returns the path to the image file' do + screenshot = double('Watir::Screenshot') + allow(browser).to receive(:screenshot).and_return(screenshot) + allow(screenshot).to receive(:save) + + expect(subject).to eq('screenshots/Some test name.png') + end + end + + describe '#teardown' do + subject { env.teardown(info) } + + let(:info) { { name: 'Some test name', status: status } } + let(:status) { :passed } + + let(:browser) { double(Watir::Browser) } + + before do + # mock {Environment#teardown} + allow(env.browser_factory).to receive(:each) { |&blk| [browser].each(&blk) } + allow(env.browser_factory).to receive(:teardown) + allow(browser).to receive(:close) + end + + it 'invokes the given block with each browser' do + allow(env).to receive(:screenshot) + + expect { |blk| env.teardown(info, &blk) }.to yield_with_args(browser) + end + + context 'configured to take screenshots of failures' do + let(:config) { { screenshot_failures: true } } + + context 'when the test case has failed' do + let(:status) { :failed } + + it 'takes a screenshot of each browser' do + expect(env).to receive(:screenshot).with(browser, 'Some test name') + subject + end + + it 'includes each screenshot in the returned artifacts' do + allow(env).to receive(:screenshot).and_return('Some test name.png') + + expect(subject).to include('Some test name.png') + expect(subject['Some test name.png']).to eq('image/png') + end + end + + context 'when the test case has not failed' do + let(:status) { :passed } + + it 'takes no screenshot' do + expect(env).not_to receive(:screenshot) + subject + end + end + end + + context 'configured to not take screenshots of failures' do + let(:config) { { screenshot_failures: false } } + let(:status) { :failed } + + it 'takes no screenshot' do + expect(env).not_to receive(:screenshot) + subject + end + end + end + end +end -- To view, visit https://gerrit.wikimedia.org/r/230230 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4534fcb05180419ee9aa354f12616aadec160659 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/selenium Gerrit-Branch: master Gerrit-Owner: Dduvall <dduv...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits