Add test for HttpIO

lib/gitlab/ci/trace/http_io.rb → ee/lib/gitlab/ci/trace/http_io.rb

@@ -7,6 +7,8 @@ module Gitlab
       class HttpIO
         BUFFER_SIZE = 128.kilobytes
 
+        FailedToGetChunkError = Class.new(StandardError)
+
         attr_reader :uri, :size
         attr_reader :tell
         attr_reader :chunk, :chunk_range
@@ -20,12 +22,17 @@ module Gitlab
         end
 
         def close
+          # no-op
         end
 
         def binmode
           # no-op
         end
 
+        def binmode?
+          true
+        end
+
         def path
           @uri.to_s
         end
@@ -53,8 +60,10 @@ module Gitlab
         end
 
         def each_line
-          loop !eof? do
+          until eof?
             line = readline
+            break if line.nil?
+
             yield(line)
           end
         end
@@ -62,7 +71,7 @@ module Gitlab
         def read(length = nil)
           out = ""
 
-          while length.nil? || out.length < length
+          until eof? || (length && out.length >= length)
             data = get_chunk
             break if data.empty?
 
@@ -70,18 +79,17 @@ module Gitlab
             @tell += data.bytesize
           end
 
-          if length && out.length > length
-            extra = out.length - length
-            out = out[0..-extra]
-          end
+          out = out[0, length] if length && out.length > length
 
           out
+        rescue FailedToGetChunkError
+          nil
         end
 
         def readline
           out = ""
 
-          loop !eof? do
+          until eof?
             data = get_chunk
             new_line = data.index("\n")
 
@@ -96,18 +104,20 @@ module Gitlab
           end
 
           out
+        rescue FailedToGetChunkError
+          nil
         end
 
         def write(data)
-          throw NotImplementedException
+          raise NotImplementedError
         end
 
         def truncate(offset)
-          throw NotImplementedException
+          raise NotImplementedError
         end
 
         def flush
-          throw NotImplementedException
+          raise NotImplementedError
         end
 
         def present?
@@ -129,6 +139,8 @@ module Gitlab
               http.request(request)
             end
 
+            raise FailedToGetChunkError unless response.code == '200'
+
             @chunk = response.body.force_encoding(Encoding::BINARY)
             @chunk_range = response.content_range
           end

spec/ee/spec/lib/gitlab/ci/trace/http_io.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Trace::HttpIO do
+  include HttpIOHelpers
+
+  let(:http_io) { described_class.new(url, size) }
+  let(:url) { remote_trace_url }
+  let(:size) { remote_trace_size }
+
+  describe 'Interchangeability between IO and HttpIO' do
+    EXCEPT_METHODS = %i[read_nonblock raw raw! cooked cooked! getch echo= echo?
+      winsize winsize= iflush oflush ioflush beep goto cursor cursor= pressed?
+      getpass write_nonblock stat pathconf wait_readable wait_writable getbyte <<
+      wait lines bytes chars codepoints getc readpartial set_encoding printf print
+      putc puts readlines gets each each_byte each_char each_codepoint to_io reopen
+      syswrite to_i fileno sysread fdatasync fsync sync= sync lineno= lineno readchar
+      ungetbyte readbyte ungetc nonblock= nread rewind pos= eof close_on_exec?
+      close_on_exec= closed? close_read close_write isatty tty? binmode? sysseek
+      advise ioctl fcntl pid external_encoding internal_encoding autoclose? autoclose=
+      posix_fileno nonblock? ready? noecho nonblock].freeze
+
+    it 'HttpIO covers core interfaces in IO' do
+      expected_interfaces = ::IO.instance_methods(false)
+      expected_interfaces = expected_interfaces - EXCEPT_METHODS
+
+      expect(expected_interfaces - described_class.instance_methods).to be_empty
+    end
+  end
+
+  describe '#close' do
+    subject { http_io.close }
+
+    it { is_expected.to be_nil }
+  end
+
+  describe '#binmode' do
+    subject { http_io.binmode }
+
+    it { is_expected.to be_nil }
+  end
+
+  describe '#binmode?' do
+    subject { http_io.binmode? }
+
+    it { is_expected.to be_truthy }
+  end
+
+  describe '#path' do
+    subject { http_io.path }
+
+    it { is_expected.to eq(url) }
+  end
+
+  describe '#seek' do
+    subject { http_io.seek(pos, where) }
+
+    context 'when moves pos to end of the file' do
+      let(:pos) { 0 }
+      let(:where) { IO::SEEK_END }
+
+      it { is_expected.to eq(size) }
+    end
+
+    context 'when moves pos to middle of the file' do
+      let(:pos) { size/2 }
+      let(:where) { IO::SEEK_SET }
+
+      it { is_expected.to eq(size/2) }
+    end
+
+    context 'when moves pos around' do
+      it 'matches the result' do
+        expect(http_io.seek(0)).to eq(0)
+        expect(http_io.seek(100, IO::SEEK_CUR)).to eq(100)
+        expect { http_io.seek(size + 1, IO::SEEK_CUR) }.to raise_error('new position is outside of file')
+      end
+    end
+  end
+
+  describe '#eof?' do
+    subject { http_io.eof? }
+
+    context 'when current pos is at end of the file' do
+      before do
+        http_io.seek(size, IO::SEEK_SET)
+      end
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when current pos is not at end of the file' do
+      before do
+        http_io.seek(0, IO::SEEK_SET)
+      end
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
+  describe '#each_line' do
+    subject { http_io.each_line }
+
+    let(:string_io) { StringIO.new(remote_trace_body) }
+
+    before do
+      stub_remote_trace_ok
+    end
+
+    it 'yields lines' do
+      expect { |b| http_io.each_line(&b) }.to yield_successive_args(*string_io.each_line.to_a)
+    end
+  end
+
+  describe '#read' do
+    subject { http_io.read(length) }
+
+    context 'when there are no network issue' do
+      before do
+        stub_remote_trace_ok
+      end
+
+      context 'when read whole size' do
+        let(:length) { nil }
+
+        context 'when BUFFER_SIZE is smaller than file size' do
+          before do
+            set_smaller_buffer_size_than(size)
+          end
+
+          it 'reads a trace' do
+            is_expected.to eq(remote_trace_body)
+          end
+        end
+
+        context 'when BUFFER_SIZE is larger than file size' do
+          before do
+            set_larger_buffer_size_than(size)
+          end
+
+          it 'reads a trace' do
+            is_expected.to eq(remote_trace_body)
+          end
+        end
+      end
+
+      context 'when read only first 100 bytes' do
+        let(:length) { 100 }
+
+        context 'when BUFFER_SIZE is smaller than file size' do
+          before do
+            set_smaller_buffer_size_than(size)
+          end
+
+          it 'reads a trace' do
+            is_expected.to eq(remote_trace_body[0, length])
+          end
+        end
+
+        context 'when BUFFER_SIZE is larger than file size' do
+          before do
+            set_larger_buffer_size_than(size)
+          end
+
+          it 'reads a trace' do
+            is_expected.to eq(remote_trace_body[0, length])
+          end
+        end
+      end
+
+      context 'when tries to read oversize' do
+        let(:length) { size + 1000 }
+
+        context 'when BUFFER_SIZE is smaller than file size' do
+          before do
+            set_smaller_buffer_size_than(size)
+          end
+
+          it 'reads a trace' do
+            is_expected.to eq(remote_trace_body)
+          end
+        end
+
+        context 'when BUFFER_SIZE is larger than file size' do
+          before do
+            set_larger_buffer_size_than(size)
+          end
+
+          it 'reads a trace' do
+            is_expected.to eq(remote_trace_body)
+          end
+        end
+      end
+
+      context 'when tries to read 0 bytes' do
+        let(:length) { 0 }
+
+        context 'when BUFFER_SIZE is smaller than file size' do
+          before do
+            set_smaller_buffer_size_than(size)
+          end
+
+          it 'reads a trace' do
+            is_expected.to be_empty
+          end
+        end
+
+        context 'when BUFFER_SIZE is larger than file size' do
+          before do
+            set_larger_buffer_size_than(size)
+          end
+
+          it 'reads a trace' do
+            is_expected.to be_empty
+          end
+        end
+      end
+    end
+
+    context 'when there is anetwork issue' do
+      let(:length) { nil }
+
+      before do
+        stub_remote_trace_ng
+      end
+
+      it 'reads a trace' do
+        is_expected.to be_nil
+      end
+    end
+  end
+
+  describe '#readline' do
+    subject { http_io.readline }
+
+    let(:string_io) { StringIO.new(remote_trace_body) }
+
+    before do
+      stub_remote_trace_ok
+    end
+
+    shared_examples 'all line matching' do
+      it 'reads a line' do
+        (0...remote_trace_body.lines.count).each do
+          expect(http_io.readline).to eq(string_io.readline)
+        end
+      end
+    end
+
+    context 'when there is anetwork issue' do
+      let(:length) { nil }
+
+      before do
+        stub_remote_trace_ng
+      end
+
+      it 'reads a trace' do
+        is_expected.to be_nil
+      end
+    end
+
+    context 'when BUFFER_SIZE is smaller than file size' do
+      before do
+        set_smaller_buffer_size_than(size)
+      end
+
+      it_behaves_like 'all line matching'
+    end
+
+    context 'when BUFFER_SIZE is larger than file size' do
+      before do
+        set_larger_buffer_size_than(size)
+      end
+
+      it_behaves_like 'all line matching'
+    end
+
+    context 'when pos is at middle of the file' do
+      before do
+        set_smaller_buffer_size_than(size)
+
+        http_io.seek(size/2)
+        string_io.seek(size/2)
+      end
+
+      it 'reads from pos' do
+        expect(http_io.readline).to eq(string_io.readline)
+      end
+    end
+  end
+
+  describe '#write' do
+    subject { http_io.write(nil) }
+
+    it { expect { subject }.to raise_error(NotImplementedError) }
+  end
+
+  describe '#truncate' do
+    subject { http_io.truncate(nil) }
+
+    it { expect { subject }.to raise_error(NotImplementedError) }
+  end
+
+  describe '#flush' do
+    subject { http_io.flush }
+
+    it { expect { subject }.to raise_error(NotImplementedError) }
+  end
+
+  describe '#present?' do
+    subject { http_io.present? }
+
+    it { is_expected.to be_truthy }
+  end
+end

spec/support/http_io/http_io_helpers.rb

+module HttpIOHelpers
+  def stub_remote_trace_ok
+    WebMock.stub_request(:get, remote_trace_url)
+      .to_return { |request| remote_trace_response(request) }
+  end
+
+  def stub_remote_trace_ng
+    WebMock.stub_request(:get, remote_trace_url)
+      .to_return(status: [500, "Internal Server Error"])
+  end
+
+  def remote_trace_url
+    "http://trace.com/trace"
+  end
+
+  def remote_trace_response(request)
+    range = request.headers['Range'].match(/bytes=(\d+)-(\d+)/)
+
+    {
+      status: 200,
+      headers: { 'Content-Type' => 'text/plain' },
+      body: range_trace_body(range[1].to_i, range[2].to_i)
+    }
+  end
+
+  def range_trace_body(from ,to)
+    remote_trace_body[from..to]
+  end
+
+  def remote_trace_body
+    @remote_trace_body ||= File.read(expand_fixture_path('trace/sample_trace'))
+  end
+
+  def remote_trace_size
+    remote_trace_body.length
+  end
+
+  def set_smaller_buffer_size_than(file_size)
+    blocks = (file_size / 128)
+    new_size = (blocks / 2) * 128
+    stub_const("Gitlab::Ci::Trace::HttpIO::BUFFER_SIZE", new_size)
+  end
+
+  def set_larger_buffer_size_than(file_size)
+    blocks = (file_size / 128)
+    new_size = (blocks * 2) * 128
+    stub_const("Gitlab::Ci::Trace::HttpIO::BUFFER_SIZE", new_size)
+  end
+end