diff --git a/spec/lib/runner/strategy/docker_container_pool_spec.rb b/spec/lib/runner/strategy/docker_container_pool_spec.rb index 67c9b20f..d5c5a455 100644 --- a/spec/lib/runner/strategy/docker_container_pool_spec.rb +++ b/spec/lib/runner/strategy/docker_container_pool_spec.rb @@ -1,15 +1,278 @@ # frozen_string_literal: true require 'rails_helper' +require 'pathname' describe Runner::Strategy::DockerContainerPool do let(:runner_id) { FactoryBot.attributes_for(:runner)[:runner_id] } let(:execution_environment) { FactoryBot.create :ruby } let(:container_pool) { described_class.new(runner_id, execution_environment) } + let(:docker_container_pool_url) { 'http://localhost:1234' } + let(:config) { {pool: {location: docker_container_pool_url}} } + let(:container) { instance_double(Docker::Container) } - # TODO: add tests for these methods when implemented - it 'defines all methods all runner management strategies must define' do - expect(container_pool.public_methods).to include(:destroy_at_management, :copy_files, :attach_to_execution) - expect(described_class.public_methods).to include(:request_from_management) + before do + allow(described_class).to receive(:config).and_return(config) + allow(container).to receive(:id).and_return(runner_id) + end + + # All requests handle a Faraday error the same way. + shared_examples 'Faraday error handling' do + it 'raises a runner error' do + allow(Faraday).to receive(:get).and_raise(Faraday::TimeoutError) + expect { action.call }.to raise_error(Runner::Error::Unknown, /Faraday/) + end + end + + describe '::request_from_management' do + let(:action) { -> { described_class.request_from_management(execution_environment) } } + let(:response_body) { nil } + let!(:request_runner_stub) do + WebMock + .stub_request(:get, "#{docker_container_pool_url}/docker_container_pool/get_container/#{execution_environment.id}") + .to_return(body: response_body, status: 200) + end + + context 'when the DockerContainerPool returns an id' do + let(:response_body) { {id: runner_id}.to_json } + + it 'successfully requests the DockerContainerPool' do + action.call + expect(request_runner_stub).to have_been_requested.once + end + + it 'returns the received runner id' do + id = action.call + expect(id).to eq(runner_id) + end + end + + context 'when the DockerContainerPool does not return an id' do + let(:response_body) { {}.to_json } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::NotAvailable) + end + end + + context 'when the DockerContainerPool returns invalid JSON' do + let(:response_body) { '{hello}' } + + it 'raises an error' do + expect { action.call }.to raise_error(Runner::Error::Unknown) + end + end + + include_examples 'Faraday error handling' + end + + describe '#destroy_at_management' do + let(:action) { -> { container_pool.destroy_at_management } } + let!(:destroy_runner_stub) do + WebMock + .stub_request(:get, "#{docker_container_pool_url}/docker_container_pool/destroy_container/#{runner_id}") + .to_return(body: nil, status: 200) + end + + before { allow(container_pool).to receive(:container).and_return(container) } + + it 'successfully requests the DockerContainerPool' do + action.call + expect(destroy_runner_stub).to have_been_requested.once + end + + include_examples 'Faraday error handling' + end + + describe '#copy_files' do + let(:files) { [] } + let(:action) { -> { container_pool.copy_files(files) } } + let(:local_path) { Pathname.new('/tmp/container20') } + + before do + allow(container_pool).to receive(:local_workspace_path).and_return(local_path) + allow(container_pool).to receive(:clean_workspace) + allow(FileUtils).to receive(:chmod_R) + end + + it 'creates the workspace directory' do + expect(FileUtils).to receive(:mkdir_p).with(local_path) + container_pool.copy_files(files) + end + + it 'cleans the workspace' do + expect(container_pool).to receive(:clean_workspace) + container_pool.copy_files(files) + end + + it 'sets permission bits on the workspace' do + expect(FileUtils).to receive(:chmod_R).with('+rwX', local_path) + container_pool.copy_files(files) + end + + context 'when receiving a normal file' do + let(:file_content) { 'print("Hello World!")' } + let(:files) { [FactoryBot.build(:file, content: file_content)] } + + it 'writes the file to disk' do + file = instance_double(File) + allow(File).to receive(:open).and_yield(file) + expect(file).to receive(:write).with(file_content) + container_pool.copy_files(files) + end + + it 'creates the file inside the workspace' do + expect(File).to receive(:open).with(local_path.join(files.first.filepath), 'w') + container_pool.copy_files(files) + end + + it 'raises an error in case of an IOError' do + allow(File).to receive(:open).and_raise(IOError) + expect { container_pool.copy_files(files) }.to raise_error(Runner::Error::Unknown, /#{files.first.filepath}/) + end + + it 'does not create a directory for it' do + expect(FileUtils).not_to receive(:mkdir_p) + end + + context 'when the file is inside a directory' do + let(:directory) { 'temp/dir' } + let(:files) { [FactoryBot.build(:file, path: directory)] } + + before do + allow(File).to receive(:open) + allow(FileUtils).to receive(:mkdir_p).with(local_path) + allow(FileUtils).to receive(:mkdir_p).with(local_path.join(directory)) + end + + it 'cleans the directory path' do + allow(container_pool).to receive(:local_path).and_call_original + expect(container_pool).to receive(:local_path).with(directory).and_call_original + container_pool.copy_files(files) + end + + it 'creates the directory of the file' do + expect(FileUtils).to receive(:mkdir_p).with(local_path.join(directory)) + container_pool.copy_files(files) + end + end + end + + context 'when receiving a binary file' do + let(:files) { [FactoryBot.build(:file, :image)] } + + it 'copies the file inside the workspace' do + expect(FileUtils).to receive(:cp).with(files.first.native_file.path, local_path.join(files.first.filepath)) + container_pool.copy_files(files) + end + end + + context 'when receiving multiple files' do + let(:files) { FactoryBot.build_list(:file, 3) } + + it 'creates all files' do + files.each do |file| + expect(File).to receive(:open).with(local_path.join(file.filepath), 'w') + end + container_pool.copy_files(files) + end + end + end + + describe '#local_workspace_path' do + before { allow(container_pool).to receive(:container).and_return(container) } + + it 'returns the local part of the mount binding' do + local_path = 'tmp/container20' + allow(container).to receive(:binds).and_return(["#{local_path}:/workspace"]) + expect(container_pool.send(:local_workspace_path)).to eq(Pathname.new(local_path)) + end + end + + describe '#local_path' do + let(:local_workspace) { Pathname.new('/tmp/workspace') } + + before { allow(container_pool).to receive(:local_workspace_path).and_return(local_workspace) } + + it 'raises an error for relative paths outside of the workspace' do + expect { container_pool.send(:local_path, '../exercise.py') }.to raise_error(Runner::Error::Unknown, %r{tmp/exercise.py}) + end + + it 'raises an error for absolute paths outside of the workspace' do + expect { container_pool.send(:local_path, '/test') }.to raise_error(Runner::Error::Unknown, %r{/test}) + end + + it 'removes .. from the path' do + expect(container_pool.send(:local_path, 'test/../exercise.py')).to eq(Pathname.new('/tmp/workspace/exercise.py')) + end + + it 'joins the given path with the local workspace path' do + expect(container_pool.send(:local_path, 'exercise.py')).to eq(Pathname.new('/tmp/workspace/exercise.py')) + end + end + + describe '#clean_workspace' do + let(:local_workspace) { instance_double(Pathname) } + + before { allow(container_pool).to receive(:local_workspace_path).and_return(local_workspace) } + + it 'removes all children of the workspace recursively' do + children = %w[test.py exercise.rb subfolder].map {|child| Pathname.new(child) } + allow(local_workspace).to receive(:children).and_return(children) + expect(FileUtils).to receive(:rm_r).with(children, secure: true) + container_pool.send(:clean_workspace) + end + + it 'raises an error if the workspace does not exist' do + allow(local_workspace).to receive(:children).and_raise(Errno::ENOENT) + expect { container_pool.send(:clean_workspace) }.to raise_error(Runner::Error::Unknown, /not exist/) + end + + it 'raises an error if it lacks permission for deleting an entry' do + allow(local_workspace).to receive(:children).and_return(['test.py']) + allow(FileUtils).to receive(:remove_entry_secure).and_raise(Errno::EACCES) + expect { container_pool.send(:clean_workspace) }.to raise_error(Runner::Error::Unknown, /Not allowed/) + end + end + + describe '#container' do + it 'raises an error if there is no container for the saved id' do + allow(Docker::Container).to receive(:get).and_raise(Docker::Error::NotFoundError) + expect { container_pool.send(:container) }.to raise_error(Runner::Error::RunnerNotFound) + end + + it 'raises an error if the received container is not running' do + allow(Docker::Container).to receive(:get).and_return(container) + allow(container).to receive(:info).and_return({'State' => {'Running' => false}}) + expect { container_pool.send(:container) }.to raise_error(Runner::Error::RunnerNotFound) + end + + it 'returns the received container' do + allow(Docker::Container).to receive(:get).and_return(container) + allow(container).to receive(:info).and_return({'State' => {'Running' => true}}) + expect(container_pool.send(:container)).to eq(container) + end + + it 'does not request a container if one is saved' do + container_pool.instance_variable_set(:@container, container) + expect(Docker::Container).not_to receive(:get) + container_pool.send(:container) + end + end + + describe '#attach_to_execution' do + # TODO: add more tests here + + let(:command) { 'ls' } + let(:action) { -> { container_pool.attach_to_execution(command) } } + let(:websocket_url) { 'ws://ws.example.com/path/to/websocket' } + + it 'returns the execution time' do + allow(EventMachine).to receive(:run) + starting_time = Time.zone.now + execution_time = action.call + test_time = Time.zone.now - starting_time + expect(execution_time).to be_between(0.0, test_time).exclusive + end end end diff --git a/spec/lib/runner/strategy/poseidon_spec.rb b/spec/lib/runner/strategy/poseidon_spec.rb index 62bbd638..add7037e 100644 --- a/spec/lib/runner/strategy/poseidon_spec.rb +++ b/spec/lib/runner/strategy/poseidon_spec.rb @@ -292,7 +292,7 @@ describe Runner::Strategy::Poseidon do # TODO: add more tests here let(:command) { 'ls' } - let(:action) { -> { poseidon.attach_to_execution command } } + let(:action) { -> { poseidon.attach_to_execution(command) } } let(:websocket_url) { 'ws://ws.example.com/path/to/websocket' } it 'returns the execution time' do @@ -302,7 +302,7 @@ describe Runner::Strategy::Poseidon do starting_time = Time.zone.now execution_time = action.call test_time = Time.zone.now - starting_time - expect(execution_time).to be_between(0.0, test_time) + expect(execution_time).to be_between(0.0, test_time).exclusive end end end diff --git a/spec/models/runner_spec.rb b/spec/models/runner_spec.rb index b9395645..771f2b4d 100644 --- a/spec/models/runner_spec.rb +++ b/spec/models/runner_spec.rb @@ -98,7 +98,8 @@ describe Runner do runner.copy_files(nil) end - it 'retries to copy the files' do + it 'calls copy_file twice' do + # copy_files is called again after a new runner was requested. expect(strategy).to receive(:copy_files).twice runner.copy_files(nil) end