--publish: use banner provided by the reports server (#1483)

* Ensure_io needs an error stream to output the potential banner * Report banner from server * http_io: do not use the request body in the thrown error has it is displayed before as a banner
cucumber · Oct 21, 2020 · e318dfc · e318dfc
1 parent a794d07
commit e318dfc
Show file tree

Hide file tree

Showing 14 changed files with 89 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,7 +16,7 @@ Please visit [cucumber/CONTRIBUTING.md](https://github.com/cucumber/cucumber/blo
 
 ### Changed
 
-* `--publish` request errors now include the response's body in the error message
+* `--publish` uses the response provided by the server as the banner [#1472](https://github.com/cucumber/cucumber-ruby/issues/1472)
 
 ### Removed
 

diff --git a/lib/cucumber/formatter/html.rb b/lib/cucumber/formatter/html.rb
@@ -8,7 +8,7 @@ class HTML < MessageBuilder
       include Io
 
       def initialize(config)
-        @io = ensure_io(config.out_stream)
+        @io = ensure_io(config.out_stream, config.error_stream)
         @html_formatter = Cucumber::HTMLFormatter::Formatter.new(@io)
         @html_formatter.write_pre_message
 

diff --git a/lib/cucumber/formatter/http_io.rb b/lib/cucumber/formatter/http_io.rb
@@ -72,10 +72,11 @@ def initialize(uri, method, headers = {}, https_verify_mode = nil, reporter = ni
       end
 
       def close
-        resource_uri = send_content(@uri, @method, @headers)
-
-        @reporter.report(resource_uri)
+        response = send_content(@uri, @method, @headers)
+        @reporter.report(response.body)
         @write_io.close
+        return if response.is_a?(Net::HTTPSuccess) || response.is_a?(Net::HTTPRedirection)
+        raise StandardError, "request to #{uri} failed with status #{response.code}"
       end
 
       def write(data)
@@ -117,16 +118,11 @@ def send_content(uri, method, headers, attempt = 10)
 
         case response
         when Net::HTTPAccepted
-          return uri unless response['Location']
-
-          send_content(URI(response['Location']), 'PUT', {}, attempt - 1)
-        when Net::HTTPSuccess
-          uri
+          send_content(URI(response['Location']), 'PUT', {}, attempt - 1) if response['Location']
         when Net::HTTPRedirection
           send_content(URI(response['Location']), method, headers, attempt - 1)
-        else
-          raise StandardError, "request to #{uri} failed with status #{response.code}: #{response.body}"
         end
+        response
       end
 
       def build_request(uri, method, headers)

diff --git a/lib/cucumber/formatter/io.rb b/lib/cucumber/formatter/io.rb
@@ -9,13 +9,13 @@ module Formatter
     module Io
       module_function
 
-      def ensure_io(path_or_url_or_io)
+      def ensure_io(path_or_url_or_io, error_stream)
         return nil if path_or_url_or_io.nil?
         return path_or_url_or_io if io?(path_or_url_or_io)
 
         io = if url?(path_or_url_or_io)
                url = path_or_url_or_io
-               reporter = url.start_with?(Cucumber::Cli::Options::CUCUMBER_PUBLISH_URL) ? URLReporter.new($stderr) : NoReporter.new
+               reporter = url.start_with?(Cucumber::Cli::Options::CUCUMBER_PUBLISH_URL) ? URLReporter.new(error_stream) : NoReporter.new
                HTTPIO.open(url, nil, reporter)
              else
                File.open(path_or_url_or_io, Cucumber.file_mode('w'))
@@ -64,7 +64,7 @@ def ensure_file(path, name)
         raise "You *must* specify --out FILE for the #{name} formatter" unless String == path.class
         raise "I can't write #{name} to a directory - it has to be a file" if File.directory?(path)
         raise "I can't write #{name} to a file in the non-existing directory #{File.dirname(path)}" unless File.directory?(File.dirname(path))
-        ensure_io(path)
+        ensure_io(path, nil)
       end
 
       def ensure_dir(path, name)

diff --git a/lib/cucumber/formatter/json.rb b/lib/cucumber/formatter/json.rb
@@ -22,7 +22,7 @@ def initialize(config)
           '6.0.0'
         )
 
-        @io = ensure_io(config.out_stream)
+        @io = ensure_io(config.out_stream, config.error_stream)
         @ast_lookup = AstLookup.new(config)
         @feature_hashes = []
         @step_or_hook_hash = {}

diff --git a/lib/cucumber/formatter/message.rb b/lib/cucumber/formatter/message.rb
@@ -10,7 +10,7 @@ class Message < MessageBuilder
       include Io
 
       def initialize(config)
-        @io = ensure_io(config.out_stream)
+        @io = ensure_io(config.out_stream, config.error_stream)
         super(config)
       end
 

diff --git a/lib/cucumber/formatter/pretty.rb b/lib/cucumber/formatter/pretty.rb
@@ -32,7 +32,7 @@ class Pretty # rubocop:disable Metrics/ClassLength
       private :in_scenario_outline, :print_background_steps
 
       def initialize(config)
-        @io = ensure_io(config.out_stream)
+        @io = ensure_io(config.out_stream, config.error_stream)
         @config = config
         @options = config.to_hash
         @snippets_input = []

diff --git a/lib/cucumber/formatter/progress.rb b/lib/cucumber/formatter/progress.rb
@@ -19,7 +19,7 @@ class Progress
 
       def initialize(config)
         @config = config
-        @io = ensure_io(config.out_stream)
+        @io = ensure_io(config.out_stream, config.error_stream)
         @snippets_input = []
         @undefined_parameter_types = []
         @total_duration = 0

diff --git a/lib/cucumber/formatter/rerun.rb b/lib/cucumber/formatter/rerun.rb
@@ -8,7 +8,7 @@ class Rerun
       include Formatter::Io
 
       def initialize(config)
-        @io = ensure_io(config.out_stream)
+        @io = ensure_io(config.out_stream, config.error_stream)
         @config = config
         @failures = {}
         config.on_event :test_case_finished do |event|

diff --git a/lib/cucumber/formatter/steps.rb b/lib/cucumber/formatter/steps.rb
@@ -5,7 +5,7 @@ module Formatter
     # The formatter used for <tt>--format steps</tt>
     class Steps
       def initialize(runtime, path_or_io, options)
-        @io = ensure_io(path_or_io)
+        @io = ensure_io(path_or_io, nil)
         @options = options
         @step_definition_files = collect_steps(runtime)
       end

diff --git a/lib/cucumber/formatter/summary.rb b/lib/cucumber/formatter/summary.rb
@@ -16,7 +16,7 @@ class Summary
 
       def initialize(config)
         @config = config
-        @io = ensure_io(config.out_stream)
+        @io = ensure_io(config.out_stream, config.error_stream)
         @ast_lookup = AstLookup.new(config)
         @counts = ConsoleCounts.new(@config)
         @issues = ConsoleIssues.new(@config, @ast_lookup)

diff --git a/lib/cucumber/formatter/url_reporter.rb b/lib/cucumber/formatter/url_reporter.rb
@@ -1,30 +1,17 @@
-require 'cucumber/term/banner'
-
 module Cucumber
   module Formatter
     class URLReporter
-      include Term::Banner
-
       def initialize(io)
         @io = io
       end
 
-      def report(url)
-        uri = URI(url)
-        display_banner(
-          [
-            'View your Cucumber Report at:',
-            [["https://reports.cucumber.io#{uri.path}", :cyan, :bold, :underline]],
-            '',
-            [['This report will self-destruct in 24h unless it is claimed or deleted.', :green, :bold]]
-          ],
-          @io
-        )
+      def report(banner)
+        @io.puts(banner)
       end
     end
 
     class NoReporter
-      def report(url); end
+      def report(banner); end
     end
   end
 end
diff --git a/spec/cucumber/formatter/http_io_spec.rb b/spec/cucumber/formatter/http_io_spec.rb
@@ -17,6 +17,15 @@ class ProcHandler < AbstractServlet
 
   let(:putreport_returned_location) { URI('/s3').to_s }
 
+  let(:success_banner) do
+    [
+      'View your Cucumber Report at:',
+      'https://reports.cucumber.io/reports/<some-random-uid>'
+    ].join("\n")
+  end
+
+  let(:failure_banner) { 'Oh noooo, something went horribly wrong :(' }
+
   # rubocop:disable Metrics/MethodLength
   # rubocop:disable Metrics/AbcSize
   def start_server
@@ -56,7 +65,16 @@ def start_server
       @request_count += 1
       @received_headers << req.header
       res.status = 404
-      res.body = 'Not found'
+      res.header['Content-Type'] = 'text/plain;charset=utf-8'
+      res.body = failure_banner
+    end
+
+    @server.mount_proc '/401' do |req, res|
+      @request_count += 1
+      @received_headers << req.header
+      res.status = 401
+      res.header['Content-Type'] = 'text/plain;charset=utf-8'
+      res.body = failure_banner
     end
 
     @server.mount_proc '/putreport' do |req, res|
@@ -67,7 +85,8 @@ def start_server
       if req.request_method == 'GET'
         res.status = 202 # Accepted
         res.header['location'] = putreport_returned_location if putreport_returned_location
-        res.body = ''
+        res.header['Content-Type'] = 'text/plain;charset=utf-8'
+        res.body = success_banner
       else
         res.set_redirect(
           WEBrick::HTTPStatus::TemporaryRedirect,
@@ -108,25 +127,29 @@ class DummyFormatter
 
       def initialize(config = nil); end
 
-      def ensure_io(path_or_url_or_io)
+      def ensure_io(path_or_url_or_io, error_stream)
         super
       end
     end
 
+    class DummyReporter
+      def report(banner); end
+    end
+
     describe HTTPIO do
       include_context 'an HTTP server accepting file requests'
 
       context 'created by Io#ensure_io' do
         it 'returns a IOHTTPBuffer' do
           url = start_server
-          io = DummyFormatter.new.ensure_io("#{url}/s3 -X PUT")
+          io = DummyFormatter.new.ensure_io("#{url}/s3 -X PUT", nil)
           expect(io).to be_a(Cucumber::Formatter::IOHTTPBuffer)
           io.close # Close during the test so the request is done while server still runs
         end
 
         it 'uses CurlOptionParser to pass correct options to IOHTTPBuffer' do
           url = start_server
-          io = DummyFormatter.new.ensure_io("#{url}/s3 -X GET -H 'Content-Type: text/json'")
+          io = DummyFormatter.new.ensure_io("#{url}/s3 -X GET -H 'Content-Type: text/json'", nil)
 
           expect(io.uri).to eq(URI("#{url}/s3"))
           expect(io.method).to eq('GET')
@@ -223,7 +246,7 @@ def ensure_io(path_or_url_or_io)
       it 'raises an error on close when server in unreachable' do
         io = IOHTTPBuffer.new("#{url}/404", 'PUT')
 
-        expect { io.close }.to(raise_error("request to #{url}/404 failed with status 404: Not found"))
+        expect { io.close }.to(raise_error("request to #{url}/404 failed with status 404"))
       end
 
       it 'raises an error on close when the server is unreachable' do
@@ -285,6 +308,34 @@ def ensure_io(path_or_url_or_io)
         expect(@received_headers[1]['authorization']).to eq([])
       end
 
+      it 'reports the body of the response to the reporter' do
+        reporter = DummyReporter.new
+        allow(reporter).to receive(:report)
+
+        io = IOHTTPBuffer.new("#{url}/putreport", 'GET', {}, nil, reporter)
+        io.write(sent_body)
+        io.flush
+        io.close
+
+        expect(reporter).to have_received(:report).with(success_banner)
+      end
+
+      it 'reports the body of the response to the reporter when request failed' do
+        reporter = DummyReporter.new
+        allow(reporter).to receive(:report)
+
+        begin
+          io = IOHTTPBuffer.new("#{url}/401", 'GET', {}, nil, reporter)
+          io.write(sent_body)
+          io.flush
+          io.close
+        rescue StandardError
+          # no-op
+        end
+
+        expect(reporter).to have_received(:report).with(failure_banner)
+      end
+
       context 'when the location http header is not set on 202 response' do
         let(:putreport_returned_location) { nil }
 

diff --git a/spec/cucumber/formatter/url_reporter_spec.rb b/spec/cucumber/formatter/url_reporter_spec.rb
@@ -10,19 +10,19 @@ module Formatter
       subject { URLReporter.new(io) }
 
       context '#report' do
-        it 'prints the corresponding reports.cucumber.io URL' do
-          subject.report('https://cucumber-messages-app-s3bucket-1rakuy67mtnt0.s3.eu-west-3.amazonaws.com/reports/8519cb24-d177-40f8-8484-3237532f7772?Content-Type=application%2Fx-ndjson&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAWE4HTFKNYESY7WHP%2F20200723%2Feu-west-3%2Fs3%2Faws4_request&X-Amz-Date=20200723T113756Z&X-Amz-Expires=900&X-Amz-Security-Token=IQoJb3JpZ2luX2VjELz%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FwEaCWV1LXdlc3QtMyJHMEUCIFBFzWgBcicdoOzqtjzFirjZfWmbGwj8kaYZ9PVUSrjaAiEAj9r3UFYpYTTQ40bomj3k02LyOKm3pBkhzpi7FtqCL3Qq8wEIdRABGgw0MjI4MDE3NzkzNTUiDDhfco779UaJWFc58irQAXw1p%2FLDYm2Rvw22Kv%2F8KRVdyl%2FvqQNwwfqqvdxirMRYuIp4UjznQazeybIMc%2F4QVNNsTRosHMCWa%2BY8nTWKg5oHyUYJwAES4dz9ZF%2BJEd0TODTJBgJ%2BrzIuIkT0Gfi%2BxZNbsjiXj%2FcSJ2YCK6RthGujnG744I9%2FTH5Zd3CGiCDcp7IvLNwPMaQzVim6aCwL98Xa4FRH1GYCV9X5AUZg%2BA4Cq5o48isX9J3NwwimIzYcQAfObnCtnwK92k5X9pwQVN9n5zKX5y6mjDVSrqKnYWgwk%2B3l%2BAU64AHQAvu4lxx9WknuN%2BZE03mVPghXZtOBQtL6TYC4IpWAPYbFLrYOO%2Fykqbtac1DL2zyaJbknbasInHapRbRiCfZVnc%2BDTRzUxIGr2Fwi4ElkHezqKvdV06cwaZBxTvNNYgj%2FA%2BwgRRRHOs03yu%2FsbIn2FOZmmTCyjRMU9i4Bz1AGlCKZDtQUye1Iv0RC3ngo6yx3QwCCRX9DZIuGg0tfGAwu82LdCkEvwy05seepyz9vjbO8cTmAUOTWzHlkLKF86px%2FJ8dDJmlVWaI%2BwCIIurOflmtQyMtjgUaMC5pjK3oRIg%3D%3D&X-Amz-Signature=df70b576e319e3fc0ed86118fe12ef2d7910a83b1f3ad944ccfaf4db30ed1d49&X-Amz-SignedHeaders=host')
+        it 'displays the provided string' do
+          banner = [
+            '┌──────────────────────────────────────────────────────────────────────────┐',
+            '│ View your Cucumber Report at:                                            │',
+            '│ https://reports.cucumber.io/reports/<some-random-uid>                    │',
+            '│                                                                          │',
+            '│ This report will self-destruct in 24h unless it is claimed or deleted.   │',
+            '└──────────────────────────────────────────────────────────────────────────┘'
+          ].join("\n")
+          subject.report(banner)
 
           io.rewind
-          expect(io.read).to eq([
-            "\e[1m\e[32m┌──────────────────────────────────────────────────────────────────────────┐\e[0m\e[0m",
-            "\e[1m\e[32m│\e[0m\e[0m View your Cucumber Report at:                                            \e[1m\e[32m│\e[0m\e[0m",
-            "\e[1m\e[32m│\e[0m\e[0m \e[4m\e[1m\e[36mhttps://reports.cucumber.io/reports/8519cb24-d177-40f8-8484-3237532f7772\e[0m\e[0m\e[0m \e[1m\e[32m│\e[0m\e[0m",
-            "\e[1m\e[32m│\e[0m\e[0m                                                                          \e[1m\e[32m│\e[0m\e[0m",
-            "\e[1m\e[32m│\e[0m\e[0m \e[1m\e[32mThis report will self-destruct in 24h unless it is claimed or deleted.\e[0m\e[0m   \e[1m\e[32m│\e[0m\e[0m",
-            "\e[1m\e[32m└──────────────────────────────────────────────────────────────────────────┘\e[0m\e[0m",
-            ''
-          ].join("\n"))
+          expect(io.read).to eq("#{banner}\n")
         end
       end
     end