From a13d1738e2a1663767c0d593c204626c074ab41b Mon Sep 17 00:00:00 2001 From: Hauke Klement Date: Tue, 17 Feb 2015 10:23:01 +0100 Subject: [PATCH] fixed multiple style guide violations --- app/controllers/concerns/common_behavior.rb | 2 +- app/controllers/concerns/submission_parameters.rb | 4 ++-- app/controllers/execution_environments_controller.rb | 2 +- app/controllers/sessions_controller.rb | 2 +- app/controllers/submissions_controller.rb | 2 +- app/helpers/application_helper.rb | 4 ++-- app/mailers/user_mailer.rb | 2 +- app/models/concerns/user.rb | 2 +- app/models/submission.rb | 6 ++---- config/initializers/cookies_serializer.rb | 2 +- config/initializers/mixins.rb | 2 +- db/seeds.rb | 12 +++++++----- lib/assessor.rb | 5 ++--- lib/code_ocean/config.rb | 5 ++--- lib/docker_client.rb | 8 +++----- lib/tasks/docker.rake | 4 ++-- lib/testing_framework_adapter.rb | 4 ++-- spec/concerns/lti_spec.rb | 2 +- spec/controllers/code_ocean/files_controller_spec.rb | 2 +- .../execution_environments_controller_spec.rb | 2 +- spec/controllers/exercises_controller_spec.rb | 6 +++--- spec/factories/exercise.rb | 4 ++-- spec/factories/file_type.rb | 4 +--- spec/lib/docker_container_pool_spec.rb | 6 ++---- spec/lib/junit_adapter_spec.rb | 4 ++-- spec/lib/py_unit_adapter_spec.rb | 2 +- spec/lib/rspec_adapter_spec.rb | 2 +- spec/lib/sql_result_set_comparator_adapter_spec.rb | 6 +++--- spec/models/code_ocean/file_spec.rb | 2 +- spec/models/execution_environment_spec.rb | 2 +- spec/rails_helper.rb | 6 +++--- spec/support/anonymous_controller.rb | 2 +- 32 files changed, 56 insertions(+), 64 deletions(-) diff --git a/app/controllers/concerns/common_behavior.rb b/app/controllers/concerns/common_behavior.rb index 7a13e6c8..a52c8b00 100644 --- a/app/controllers/concerns/common_behavior.rb +++ b/app/controllers/concerns/common_behavior.rb @@ -1,5 +1,5 @@ module CommonBehavior - def create_and_respond(options = {}, &block) + def create_and_respond(options = {}) @object = options[:object] respond_to do |format| if @object.save diff --git a/app/controllers/concerns/submission_parameters.rb b/app/controllers/concerns/submission_parameters.rb index 0cb1b304..f67e2f7c 100644 --- a/app/controllers/concerns/submission_parameters.rb +++ b/app/controllers/concerns/submission_parameters.rb @@ -2,8 +2,8 @@ module SubmissionParameters include FileParameters def reject_illegal_file_attributes!(submission_params) - if exercise = Exercise.find_by(id: submission_params[:exercise_id]) - submission_params[:files_attributes].try(:reject!) do |index, file_attributes| + if Exercise.exists?(id: submission_params[:exercise_id]) + submission_params[:files_attributes].try(:reject!) do |_, file_attributes| file = CodeOcean::File.find_by(id: file_attributes[:file_id]) file.nil? || file.hidden || file.read_only end diff --git a/app/controllers/execution_environments_controller.rb b/app/controllers/execution_environments_controller.rb index 8a59cba7..bd103a17 100644 --- a/app/controllers/execution_environments_controller.rb +++ b/app/controllers/execution_environments_controller.rb @@ -24,7 +24,7 @@ class ExecutionEnvironmentsController < ApplicationController end def execute_command - @docker_client = DockerClient.new(execution_environment: @execution_environment, user: current_user) + @docker_client = DockerClient.new(execution_environment: @execution_environment) render(json: @docker_client.execute_arbitrary_command(params[:command])) end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 6bf9ce4b..a2036d81 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -9,7 +9,7 @@ class SessionsController < ApplicationController skip_before_action :verify_authenticity_token, only: :create_through_lti def create - if user = login(params[:email], params[:password], params[:remember_me]) + if login(params[:email], params[:password], params[:remember_me]) redirect_back_or_to(:root, notice: t('.success')) else flash.now[:danger] = t('.failure') diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index df8b8696..b6f4fe77 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -72,7 +72,7 @@ class SubmissionsController < ApplicationController end def set_docker_client - @docker_client = DockerClient.new(execution_environment: @submission.execution_environment, user: current_user) + @docker_client = DockerClient.new(execution_environment: @submission.execution_environment) end private :set_docker_client diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4dfecf21..411a6760 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -42,7 +42,7 @@ module ApplicationHelper Kramdown::Document.new(markdown).to_html.html_safe end - def row(options={}, &block) + def row(options = {}, &block) content_tag(:div, class: 'attribute-row row') do label_column(options[:label]) + value_column(options[:value], &block) end @@ -65,7 +65,7 @@ module ApplicationHelper end private :translation_present? - def value_column(value, &block) + def value_column(value) content_tag(:div, class: 'col-sm-9') do block_given? ? yield : symbol_for(value) end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 5abf263e..4b3c71f4 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -4,7 +4,7 @@ class UserMailer < ActionMailer::Base mail(subject: t('mailers.user_mailer.activation_needed.subject'), to: user.email) end - def activation_success_email(user) + def activation_success_email(*) end def reset_password_email(user) diff --git a/app/models/concerns/user.rb b/app/models/concerns/user.rb index e934e664..5f80152e 100644 --- a/app/models/concerns/user.rb +++ b/app/models/concerns/user.rb @@ -13,7 +13,7 @@ module User end ROLES.each do |role| - define_method("#{role}?") { self.try(:role) == role } + define_method("#{role}?") { try(:role) == role } end def external? diff --git a/app/models/submission.rb b/app/models/submission.rb index dd7991fa..924aa660 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -7,6 +7,8 @@ class Submission < ActiveRecord::Base belongs_to :exercise + delegate :execution_environment, to: :exercise + scope :final, -> { where(cause: 'submit') } scope :intermediate, -> { where.not(cause: 'submit') } @@ -24,10 +26,6 @@ class Submission < ActiveRecord::Base ancestors.merge(descendants).values end - def execution_environment - exercise.execution_environment - end - [:download, :render, :run, :test].each do |action| filename = FILENAME_URL_PLACEHOLDER.gsub(/\W/, '') define_method("#{action}_url") do diff --git a/config/initializers/cookies_serializer.rb b/config/initializers/cookies_serializer.rb index 7a06a89f..7f70458d 100644 --- a/config/initializers/cookies_serializer.rb +++ b/config/initializers/cookies_serializer.rb @@ -1,3 +1,3 @@ # Be sure to restart your server when you modify this file. -Rails.application.config.action_dispatch.cookies_serializer = :json \ No newline at end of file +Rails.application.config.action_dispatch.cookies_serializer = :json diff --git a/config/initializers/mixins.rb b/config/initializers/mixins.rb index ca527e2d..9a8d8ad8 100644 --- a/config/initializers/mixins.rb +++ b/config/initializers/mixins.rb @@ -1 +1 @@ -Docker::Container::send(:include, DockerContainerMixin) +Docker::Container.send(:include, DockerContainerMixin) diff --git a/db/seeds.rb b/db/seeds.rb index d0229e81..823a6358 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -4,11 +4,13 @@ def find_factories_by_class(klass) end end -class ActiveRecord::Base - [:build, :create].each do |strategy| - define_singleton_method("#{strategy}_factories") do |attributes = {}| - find_factories_by_class(self).map(&:name).map do |factory_name| - FactoryGirl.send(strategy, factory_name, attributes) +module ActiveRecord + class Base + [:build, :create].each do |strategy| + define_singleton_method("#{strategy}_factories") do |attributes = {}| + find_factories_by_class(self).map(&:name).map do |factory_name| + FactoryGirl.send(strategy, factory_name, attributes) + end end end end diff --git a/lib/assessor.rb b/lib/assessor.rb index 177b99e1..fef25559 100644 --- a/lib/assessor.rb +++ b/lib/assessor.rb @@ -4,7 +4,7 @@ class Assessor def assess(output) test_outcome = @testing_framework_adapter.test_outcome(output) test_outcome.merge(score: calculate_score(test_outcome)) - rescue Exception + rescue {score: 0} end @@ -20,7 +20,6 @@ class Assessor fail(Error, 'No testing framework adapter set!') end end -end -class Assessor::Error < RuntimeError + class Error < RuntimeError; end end diff --git a/lib/code_ocean/config.rb b/lib/code_ocean/config.rb index 0f27f0dd..f73e54b9 100644 --- a/lib/code_ocean/config.rb +++ b/lib/code_ocean/config.rb @@ -6,15 +6,14 @@ module CodeOcean def read(options = {}) path = Rails.root.join('config', "#{@filename}.yml#{options[:erb] ? '.erb' : ''}") - if ::File.exists?(path) + if ::File.exist?(path) content = options[:erb] ? YAML.load(ERB.new(::File.new(path, 'r').read).result) : YAML.load_file(path) content[Rails.env].with_indifferent_access else fail(Error, "Configuration file not found: #{path}") end end - end - class Config::Error < RuntimeError + class Error < RuntimeError; end end end diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 7d66b3cc..777366a2 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -8,7 +8,7 @@ class DockerClient attr_reader :container_id def self.check_availability! - Timeout::timeout(config[:connection_timeout]) { Docker.version } + Timeout.timeout(config[:connection_timeout]) { Docker.version } rescue Excon::Errors::SocketError, Timeout::Error raise(Error, "The Docker host at #{Docker.url} is not reachable!") end @@ -94,7 +94,6 @@ class DockerClient def initialize(options = {}) @execution_environment = options[:execution_environment] - @user = options[:user] @image = self.class.find_image_by_tag(@execution_environment.docker_image) fail(Error, "Cannot find image #{@execution_environment.docker_image}!") unless @image end @@ -133,7 +132,7 @@ class DockerClient end def send_command(command, container, &block) - Timeout::timeout(@execution_environment.permitted_execution_time) do + Timeout.timeout(@execution_environment.permitted_execution_time) do stderr = [] stdout = [] container.attach(stdin: StringIO.new(command)) do |stream, chunk| @@ -152,7 +151,6 @@ class DockerClient Concurrent::Future.execute { self.class.destroy_container(container) } end private :send_command -end -class DockerClient::Error < RuntimeError + class Error < RuntimeError; end end diff --git a/lib/tasks/docker.rake b/lib/tasks/docker.rake index cc5b67fb..5152429a 100644 --- a/lib/tasks/docker.rake +++ b/lib/tasks/docker.rake @@ -6,12 +6,12 @@ namespace :docker do end desc 'List all installed Docker images' - task :images => :environment do + task images: :environment do puts DockerClient.image_tags end desc 'Pull all Docker images referenced by execution environments' - task :pull => :environment do + task pull: :environment do ExecutionEnvironment.all.map(&:docker_image).each do |docker_image| puts "Pulling #{docker_image}..." DockerClient.pull(docker_image) diff --git a/lib/testing_framework_adapter.rb b/lib/testing_framework_adapter.rb index 01d01d7e..895a5299 100644 --- a/lib/testing_framework_adapter.rb +++ b/lib/testing_framework_adapter.rb @@ -11,10 +11,10 @@ class TestingFrameworkAdapter private :augment_output def self.framework_name - self.name + name end - def parse_output(output) + def parse_output(*) fail(NotImplementedError, "#{self.class} should implement #parse_output!") end private :parse_output diff --git a/spec/concerns/lti_spec.rb b/spec/concerns/lti_spec.rb index 93f034de..09d898bf 100644 --- a/spec/concerns/lti_spec.rb +++ b/spec/concerns/lti_spec.rb @@ -67,7 +67,7 @@ describe Lti do describe '#return_to_consumer' do context 'with a return URL' do let(:consumer_return_url) { 'http://example.org' } - before(:each) { expect(controller).to receive(:params).and_return({launch_presentation_return_url: consumer_return_url}) } + before(:each) { expect(controller).to receive(:params).and_return(launch_presentation_return_url: consumer_return_url) } it 'redirects to the tool consumer' do expect(controller).to receive(:redirect_to).with(consumer_return_url) diff --git a/spec/controllers/code_ocean/files_controller_spec.rb b/spec/controllers/code_ocean/files_controller_spec.rb index cedb1d5c..3adb76be 100644 --- a/spec/controllers/code_ocean/files_controller_spec.rb +++ b/spec/controllers/code_ocean/files_controller_spec.rb @@ -38,7 +38,7 @@ describe CodeOcean::FilesController do expect_assigns(file: CodeOcean::File) it 'destroys the file' do - exercise = FactoryGirl.create(:fibonacci) + FactoryGirl.create(:fibonacci) expect { request.call }.to change(CodeOcean::File, :count).by(-1) end diff --git a/spec/controllers/execution_environments_controller_spec.rb b/spec/controllers/execution_environments_controller_spec.rb index 31869490..6a6b4b64 100644 --- a/spec/controllers/execution_environments_controller_spec.rb +++ b/spec/controllers/execution_environments_controller_spec.rb @@ -60,7 +60,7 @@ describe ExecutionEnvironmentsController do let(:command) { 'which ruby' } before(:each) do - expect(DockerClient).to receive(:new).with(execution_environment: execution_environment, user: user).and_call_original + expect(DockerClient).to receive(:new).with(execution_environment: execution_environment).and_call_original expect_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).with(command) post :execute_command, command: command, id: execution_environment.id end diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index a2358508..e70df41d 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -30,7 +30,7 @@ describe ExercisesController do end end - context "with a file upload" do + context 'with a file upload' do let(:files_attributes) { {'0' => FactoryGirl.build(:file, content: fixture_file_upload('upload.rb', 'text/x-ruby')).attributes} } let(:request) { proc { post :create, exercise: exercise_attributes.merge(files_attributes: files_attributes) } } @@ -144,7 +144,7 @@ describe ExercisesController do context 'when the score transmission succeeds' do before(:each) do - expect(controller).to receive(:send_score).and_return({status: 'success'}) + expect(controller).to receive(:send_score).and_return(status: 'success') request end @@ -160,7 +160,7 @@ describe ExercisesController do context 'when the score transmission fails' do before(:each) do - expect(controller).to receive(:send_score).and_return({status: 'unsupported'}) + expect(controller).to receive(:send_score).and_return(status: 'unsupported') request end diff --git a/spec/factories/exercise.rb b/spec/factories/exercise.rb index dbbacd57..ea588faf 100644 --- a/spec/factories/exercise.rb +++ b/spec/factories/exercise.rb @@ -10,7 +10,7 @@ def create_seed_file(exercise, path, file_attributes = {}) else file_attributes.merge!(content: SeedsHelper.read_seed_file(path)) end - file = exercise.add_file!(file_attributes) + exercise.add_file!(file_attributes) end FactoryGirl.define do @@ -18,7 +18,7 @@ FactoryGirl.define do created_by_teacher description "Try HTML's audio and video capabilities." association :execution_environment, factory: :html - instructions "Build a simple website including an HTML