From da0a682ffb3acc77043eae8c7d3f4f43bb5e568b Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Fri, 14 May 2021 11:07:11 +0200 Subject: [PATCH] Apply manual rubocop fixes --- app/controllers/application_controller.rb | 2 +- .../code_ocean/files_controller.rb | 5 +- .../codeharbor_links_controller.rb | 2 +- app/controllers/comments_controller.rb | 2 +- app/controllers/concerns/lti.rb | 7 +-- .../concerns/remote_evaluation_parameters.rb | 3 +- .../concerns/submission_scoring.rb | 2 +- .../exercise_collections_controller.rb | 4 +- app/controllers/exercises_controller.rb | 21 +++---- app/controllers/flowr_controller.rb | 8 +-- .../request_for_comments_controller.rb | 8 +-- app/controllers/submissions_controller.rb | 10 +-- .../user_exercise_feedbacks_controller.rb | 4 +- app/helpers/application_helper.rb | 2 + app/mailers/application_mailer.rb | 4 ++ app/models/concerns/default_values.rb | 2 + app/models/exercise.rb | 7 +-- app/models/exercise_collection.rb | 4 +- app/models/intervention.rb | 2 +- app/models/proxy_exercise.rb | 44 ++++++------- app/models/submission.rb | 2 +- app/models/tag.rb | 6 +- app/policies/exercise_policy.rb | 2 +- app/policies/request_for_comment_policy.rb | 4 +- .../exercise_service/check_external.rb | 1 + .../exercise_service/push_external.rb | 1 + .../convert_exercise_to_task.rb | 1 + .../convert_task_to_exercise.rb | 1 + app/services/proforma_service/export_task.rb | 1 + app/services/proforma_service/import.rb | 1 + app/views/application/_session.html.slim | 4 +- app/views/exercises/index.html.slim | 2 +- app/views/exercises/show.html.slim | 2 +- app/views/user_mailer/got_new_comment.slim | 2 +- ...got_new_comment_for_subscription.html.slim | 2 +- config/environments/development.rb | 2 +- config/initializers/monkey_patches.rb | 10 +-- config/locales/de.yml | 8 +-- config/locales/en.yml | 8 +-- config/routes.rb | 6 +- ...add_pool_size_to_execution_environments.rb | 2 +- ..._memory_limit_to_execution_environments.rb | 2 +- ...twork_enabled_to_execution_environments.rb | 2 +- .../20170205210357_create_interventions.rb | 2 +- ..._default_for_request_for_comment_solved.rb | 2 +- .../20170830083601_add_cause_to_testruns.rb | 2 +- ...ached_full_score_to_request_for_comment.rb | 2 +- ...181119161514_add_user_to_proxy_exercise.rb | 2 +- ...user_type_to_remote_evaluation_mappings.rb | 2 +- db/migrate/20181129093207_drop_errors.rb | 2 +- ..._to_active_storage_blobs.active_storage.rb | 4 +- db/seeds.rb | 4 +- db/seeds/fibonacci/exercise.rb | 2 +- db/seeds/fibonacci/exercise_spec_3.rb | 4 +- db/seeds/fibonacci/reference.rb | 4 +- db/seeds/hello_world/exercise.rb | 3 + db/seeds/production.rb | 2 +- db/seeds/sql_select/comparator.rb | 2 + db/seeds/tdd/exercise.rb | 3 + .../validations/boolean_presence_validator.rb | 4 +- lib/assessor.rb | 2 +- lib/code_ocean/config.rb | 2 +- lib/docker_client.rb | 37 +++++------ lib/file_tree.rb | 4 +- lib/python20_course_week.rb | 8 +-- lib/tasks/detect_exercise_anomalies.rake | 5 +- spec/concerns/lti_spec.rb | 31 +++++----- spec/concerns/submission_scoring_spec.rb | 15 +++-- .../application_controller_spec.rb | 6 +- .../codeharbor_links_controller_spec.rb | 2 +- spec/controllers/consumers_controller_spec.rb | 7 ++- .../execution_environments_controller_spec.rb | 25 ++++---- spec/controllers/exercises_controller_spec.rb | 40 ++++++------ .../controllers/file_types_controller_spec.rb | 7 ++- .../internal_users_controller_spec.rb | 9 ++- .../request_for_comments_controller_spec.rb | 12 ++-- spec/controllers/sessions_controller_spec.rb | 27 +++++--- .../submissions_controller_spec.rb | 23 +++---- spec/db/seeds_spec.rb | 2 + spec/factories/code_ocean/file.rb | 2 +- spec/factories/lti_parameter.rb | 6 +- spec/features/authorization_spec.rb | 6 +- spec/features/editor_spec.rb | 2 +- spec/features/prometheus/controller_spec.rb | 3 +- spec/lib/assessor_spec.rb | 2 +- spec/lib/code_ocean/config_spec.rb | 2 +- spec/lib/docker_client_spec.rb | 45 +++++++------- spec/lib/docker_container_mixin_spec.rb | 4 +- spec/lib/file_tree_spec.rb | 28 ++++----- ...esting_framework_adapter_generator_spec.rb | 2 +- spec/lib/nonce_store_spec.rb | 6 +- spec/lib/testing_framework_adapter_spec.rb | 2 +- spec/models/code_ocean/file_spec.rb | 2 +- spec/models/execution_environment_spec.rb | 12 ++-- spec/models/exercise_spec.rb | 4 +- spec/models/submission_spec.rb | 4 +- spec/policies/admin/dashboard_policy_spec.rb | 8 +-- spec/policies/code_ocean/file_policy_spec.rb | 34 +++++----- spec/policies/consumer_policy_spec.rb | 6 +- .../execution_environment_policy_spec.rb | 20 +++--- spec/policies/exercise_policy_spec.rb | 62 +++++++++---------- spec/policies/external_user_policy_spec.rb | 12 ++-- spec/policies/file_type_policy_spec.rb | 8 +-- spec/policies/internal_user_policy_spec.rb | 12 ++-- spec/policies/submission_policy_spec.rb | 12 ++-- .../convert_task_to_exercise_spec.rb | 10 +-- spec/services/proforma_service/import_spec.rb | 2 +- spec/support/docker.rb | 2 +- spec/uploaders/file_uploader_spec.rb | 2 +- 109 files changed, 431 insertions(+), 416 deletions(-) create mode 100644 app/mailers/application_mailer.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7bc90b9d..f77ff520 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -6,7 +6,7 @@ class ApplicationController < ActionController::Base MEMBER_ACTIONS = %i[destroy edit show update].freeze - after_action :verify_authorized, except: %i[help welcome] + after_action :verify_authorized, except: %i[welcome] around_action :mnemosyne_trace before_action :set_sentry_context, :set_locale, :allow_iframe_requests, :load_embed_options protect_from_forgery(with: :exception, prepend: true) diff --git a/app/controllers/code_ocean/files_controller.rb b/app/controllers/code_ocean/files_controller.rb index 75bb8d0a..21a31664 100644 --- a/app/controllers/code_ocean/files_controller.rb +++ b/app/controllers/code_ocean/files_controller.rb @@ -31,7 +31,10 @@ module CodeOcean path: path, status: :created) else filename = "#{@object.path || ''}/#{@object.name || ''}#{@object.file_type.try(:file_extension) || ''}" - format.html { redirect_to(options[:path]); flash[:danger] = t('files.error.filename', name: filename) } + format.html do + flash[:danger] = t('files.error.filename', name: filename) + redirect_to(options[:path]) + end format.json { render(json: @object.errors, status: :unprocessable_entity) } end end diff --git a/app/controllers/codeharbor_links_controller.rb b/app/controllers/codeharbor_links_controller.rb index 019366e8..91eedb26 100644 --- a/app/controllers/codeharbor_links_controller.rb +++ b/app/controllers/codeharbor_links_controller.rb @@ -3,7 +3,7 @@ class CodeharborLinksController < ApplicationController include CommonBehavior before_action :verify_codeharbor_activation - before_action :set_codeharbor_link, only: %i[show edit update destroy] + before_action :set_codeharbor_link, only: %i[edit update destroy] def new base_url = CodeOcean::Config.new(:code_ocean).read[:codeharbor][:url] || '' diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 32c31229..8a8db63a 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class CommentsController < ApplicationController - before_action :set_comment, only: %i[show edit update destroy] + before_action :set_comment, only: %i[show update destroy] # to disable authorization check: comment the line below back in # skip_after_action :verify_authorized diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index dbbc726f..5516b7e8 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -60,11 +60,9 @@ module Lti if provider.roles.present? provider.roles.each do |role| case role.downcase - when 'administrator' + when 'administrator', 'instructor' # We don't want anyone to get admin privileges through LTI result = 'teacher' if result == 'learner' - when 'instructor' - result = 'teacher' if result == 'learner' else # 'learner' next end @@ -172,8 +170,7 @@ module Lti normalized_lit_score *= 0.0 end response = provider.post_replace_result!(normalized_lit_score) - {code: response.response_code, message: response.post_response.body, status: response.code_major, -score_sent: normalized_lit_score} + {code: response.response_code, message: response.post_response.body, status: response.code_major, score_sent: normalized_lit_score} else {status: 'unsupported'} end diff --git a/app/controllers/concerns/remote_evaluation_parameters.rb b/app/controllers/concerns/remote_evaluation_parameters.rb index 0f0c9222..5b81d177 100644 --- a/app/controllers/concerns/remote_evaluation_parameters.rb +++ b/app/controllers/concerns/remote_evaluation_parameters.rb @@ -5,8 +5,7 @@ module RemoteEvaluationParameters def remote_evaluation_params if params[:remote_evaluation].present? - remote_evaluation_params = params[:remote_evaluation].permit(:validation_token, - files_attributes: file_attributes) + params[:remote_evaluation].permit(:validation_token, files_attributes: file_attributes) end end private :remote_evaluation_params diff --git a/app/controllers/concerns/submission_scoring.rb b/app/controllers/concerns/submission_scoring.rb index 2fa4fae9..29274b7a 100644 --- a/app/controllers/concerns/submission_scoring.rb +++ b/app/controllers/concerns/submission_scoring.rb @@ -81,7 +81,7 @@ module SubmissionScoring end end submission.update(score: score) - if submission.normalized_score == 1.0 + if submission.normalized_score.to_d == 1.0.to_d Thread.new do RequestForComment.where(exercise_id: submission.exercise_id, user_id: submission.user_id, user_type: submission.user_type).each do |rfc| diff --git a/app/controllers/exercise_collections_controller.rb b/app/controllers/exercise_collections_controller.rb index b61d0334..598125dd 100644 --- a/app/controllers/exercise_collections_controller.rb +++ b/app/controllers/exercise_collections_controller.rb @@ -58,8 +58,8 @@ class ExerciseCollectionsController < ApplicationController end sanitized_params[:exercise_ids] = sanitized_params[:exercise_ids].reject {|v| v.nil? or v == '' } sanitized_params.tap do |p| - p[:exercise_collection_items] = p[:exercise_ids].map.with_index do |_id, index| - ExerciseCollectionItem.find_or_create_by(exercise_id: _id, exercise_collection_id: @exercise_collection.id, position: index) + p[:exercise_collection_items] = p[:exercise_ids].map.with_index do |id, index| + ExerciseCollectionItem.find_or_create_by(exercise_id: id, exercise_collection_id: @exercise_collection.id, position: index) end p.delete(:exercise_ids) end diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 07a75fd9..743a5588 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -61,7 +61,7 @@ raise: false def collect_paths(files) unique_paths = files.map(&:path).reject(&:blank?).uniq subpaths = unique_paths.map do |path| - (path.split('/').length + 1).times.map do |n| + Array.new((path.split('/').length + 1)) do |n| path.split('/').shift(n).join('/') end end @@ -271,7 +271,7 @@ user_id: current_user.id, user_type: current_user.class.name def implement redirect_to(@exercise, alert: t('exercises.implement.unpublished')) if @exercise.unpublished? && current_user.role != 'admin' && current_user.role != 'teacher' # TODO: TESTESTEST redirect_to(@exercise, alert: t('exercises.implement.no_files')) unless @exercise.files.visible.exists? - user_solved_exercise = @exercise.has_user_solved(current_user) + user_solved_exercise = @exercise.solved_by?(current_user) count_interventions_today = UserExerciseIntervention.where(user: current_user).where('created_at >= ?', Time.zone.now.beginning_of_day).count user_got_intervention_in_exercise = UserExerciseIntervention.where(user: current_user, @@ -280,11 +280,10 @@ exercise: @exercise).size >= max_intervention_count_per_exercise if @embed_options[:disable_interventions] @show_rfc_interventions = false - @show_break_interventions = false else @show_rfc_interventions = (!user_solved_exercise && !user_got_enough_interventions).to_s - @show_break_interventions = false end + @show_break_interventions = false @hide_rfc_button = @embed_options[:disable_rfc] @@ -308,9 +307,7 @@ exercise: @exercise).size >= max_intervention_count_per_exercise lti_json = lti_parameters.lti_parameters['launch_presentation_return_url'] @course_token = - if lti_json.nil? - '' - elsif match = lti_json.match(%r{^.*courses/([a-z0-9\-]+)/sections}) + if lti_json.present? && (match = lti_json.match(%r{^.*courses/([a-z0-9\-]+)/sections})) match.captures.first else '' @@ -470,7 +467,7 @@ working_time_accumulated: working_time_accumulated}) authorize(@external_user, :statistics?) if policy(@exercise).detailed_statistics? @submissions = Submission.where(user: @external_user, -exercise_id: @exercise.id).in_study_group_of(current_user).order('created_at') + exercise_id: @exercise.id).in_study_group_of(current_user).order('created_at') interventions = UserExerciseIntervention.where('user_id = ? AND exercise_id = ?', @external_user.id, @exercise.id) @all_events = (@submissions + interventions).sort_by(&:created_at) @@ -480,11 +477,11 @@ exercise_id: @exercise.id).in_study_group_of(current_user).order('created_at') end @working_times_until = [] @all_events.each_with_index do |_, index| - @working_times_until.push((format_time_difference(@deltas[0..index].inject(:+)) if index.positive?)) + @working_times_until.push((format_time_difference(@deltas[0..index].sum) if index.positive?)) end else final_submissions = Submission.where(user: @external_user, -exercise_id: @exercise.id).in_study_group_of(current_user).final + exercise_id: @exercise.id).in_study_group_of(current_user).final @submissions = [] %i[before_deadline within_grace_period after_late_deadline].each do |filter| relevant_submission = final_submissions.send(filter).latest @@ -570,7 +567,7 @@ normalized_score: @submission.normalized_score}) def redirect_after_submit Rails.logger.debug("Redirecting user with score:s #{@submission.normalized_score}") - if @submission.normalized_score == 1.0 + if @submission.normalized_score.to_d == 1.0.to_d # if user is external and has an own rfc, redirect to it and message him to clean up and accept the answer. (we need to check that the user is external, # otherwise an internal user could be shown a false rfc here, since current_user.id is polymorphic, but only makes sense for external users when used with rfcs.) # redirect 10 percent pseudorandomly to the feedback page @@ -603,7 +600,7 @@ normalized_score: @submission.normalized_score}) flash.keep(:notice) # increase counter 'times_featured' in rfc - rfc.increment!(:times_featured) + rfc.increment(:times_featured) clear_lti_session_data(@submission.exercise_id, @submission.user_id) respond_to do |format| diff --git a/app/controllers/flowr_controller.rb b/app/controllers/flowr_controller.rb index b507d174..e4c3c2ca 100644 --- a/app/controllers/flowr_controller.rb +++ b/app/controllers/flowr_controller.rb @@ -23,7 +23,7 @@ class FlowrController < ApplicationController # for each error get all attributes, filter out uninteresting ones, and build a query insights = errors.map do |error| attributes = error.structured_error_attributes.select do |attribute| - is_interesting(attribute) and attribute.match + interesting?(attribute) and attribute.match end # once the programming language model becomes available, the language name can be added to the query to # produce more relevant results @@ -35,8 +35,8 @@ class FlowrController < ApplicationController render json: insights, status: :ok end - def is_interesting(attribute) - attribute.error_template_attribute.key.index(/error message|error type/i) != nil + def interesting?(attribute) + !attribute.error_template_attribute.key.index(/error message|error type/i).nil? end - private :is_interesting + private :interesting? end diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb index 9330f0e0..aba8d57d 100644 --- a/app/controllers/request_for_comments_controller.rb +++ b/app/controllers/request_for_comments_controller.rb @@ -6,7 +6,7 @@ class RequestForCommentsController < ApplicationController before_action :require_user! before_action :set_request_for_comment, only: %i[show mark_as_solved set_thank_you_note] before_action :set_study_group_grouping, - only: %i[index get_my_comment_requests get_rfcs_with_my_comments get_rfcs_for_exercise] + only: %i[index my_comment_requests rfcs_with_my_comments rfcs_for_exercise] def authorize! authorize(@request_for_comments || @request_for_comment) @@ -31,7 +31,7 @@ class RequestForCommentsController < ApplicationController end # GET /my_request_for_comments - def get_my_comment_requests + def my_comment_requests @search = RequestForComment .with_last_activity .where(user: current_user) @@ -44,7 +44,7 @@ class RequestForCommentsController < ApplicationController end # GET /my_rfc_activity - def get_rfcs_with_my_comments + def rfcs_with_my_comments @search = RequestForComment .with_last_activity .joins(:comments) # we don't need to outer join here, because we know the user has commented on these @@ -58,7 +58,7 @@ class RequestForCommentsController < ApplicationController end # GET /exercises/:id/request_for_comments - def get_rfcs_for_exercise + def rfcs_for_exercise exercise = Exercise.find(params[:exercise_id]) @search = RequestForComment .with_last_activity diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 09c85239..95389e03 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -9,7 +9,7 @@ class SubmissionsController < ApplicationController include Tubesock::Hijack before_action :set_submission, - only: %i[download download_file render_file run score extract_errors show statistics stop test] + only: %i[download download_file render_file run score extract_errors show statistics test] before_action :set_docker_client, only: %i[run test] before_action :set_files, only: %i[download download_file render_file show run] before_action :set_file, only: %i[download_file render_file run] @@ -199,7 +199,7 @@ user_type: current_user.class.name, row: annotation[1][:row], column: annotation socket.send data Rails.logger.debug("Sent the received client data to docker:#{data}") end - rescue JSON::ParserError => e + rescue JSON::ParserError socket.send data Rails.logger.debug("Rescued parsing error, sent the received client data to docker:#{data}") Sentry.set_extras(data: data) @@ -266,7 +266,7 @@ user_type: current_user.class.name, row: annotation[1][:row], column: annotation end end - def parse_message(message, output_stream, socket, container = nil, recursive = true) + def parse_message(message, output_stream, socket, container = nil, recursive: true) parsed = '' begin parsed = JSON.parse(message) @@ -279,11 +279,11 @@ user_type: current_user.class.name, row: annotation[1][:row], column: annotation socket.send_data JSON.dump(parsed) Rails.logger.info("parse_message sent: #{JSON.dump(parsed)}") end - rescue JSON::ParserError => e + rescue JSON::ParserError # Check wether the message contains multiple lines, if true try to parse each line if recursive && message.include?("\n") message.split("\n").each do |part| - parse_message(part, output_stream, socket, container, false) + parse_message(part, output_stream, socket, container, recursive: false) end elsif message.include?(' #{v}" - end.to_s) + Rails.logger.debug("current users knowledge loss: #{current_users_knowledge_lack.map do |k, v| + "#{k} => #{v}" + end}") Rails.logger.debug("relative improvements #{relative_knowledge_improvement.map {|k, v| "#{k.id}:#{v}" }}") best_matching_exercise end @@ -166,22 +166,22 @@ exercise: matching_exercise, proxy_exercise: self, reason: @reason.to_json) end private :scoring_matrix_quantiles - def score(user, ex) - max_score = ex.maximum_score.to_f + def score(user, exercise) + max_score = exercise.maximum_score.to_f if max_score <= 0 - Rails.logger.debug("scoring user #{user.id} for exercise #{ex.id}: score: 0") + Rails.logger.debug("scoring user #{user.id} for exercise #{exercise.id}: score: 0") return 0.0 end - points_ratio = ex.maximum_score(user) / max_score - if points_ratio == 0.0 - Rails.logger.debug("scoring user #{user.id} for exercise #{ex.id}: points_ratio=#{points_ratio} score: 0") + points_ratio = exercise.maximum_score(user) / max_score + if points_ratio.to_d == 0.0.to_d + Rails.logger.debug("scoring user #{user.id} for exercise #{exercise.id}: points_ratio=#{points_ratio} score: 0") return 0.0 elsif points_ratio > 1.0 points_ratio = 1.0 # The score of the exercise was adjusted and is now lower than it was end points_ratio_index = ((scoring_matrix.size - 1) * points_ratio).to_i - working_time_user = ex.accumulated_working_time_for_only(user) - quantiles_working_time = ex.get_quantiles(scoring_matrix_quantiles) + working_time_user = exercise.accumulated_working_time_for_only(user) + quantiles_working_time = exercise.get_quantiles(scoring_matrix_quantiles) quantile_index = quantiles_working_time.size quantiles_working_time.each_with_index do |quantile_time, i| if working_time_user <= quantile_time @@ -190,7 +190,7 @@ exercise: matching_exercise, proxy_exercise: self, reason: @reason.to_json) end end Rails.logger.debug( - "scoring user #{user.id} exercise #{ex.id}: worktime #{working_time_user}, points: #{points_ratio}" \ + "scoring user #{user.id} exercise #{exercise.id}: worktime #{working_time_user}, points: #{points_ratio}" \ "(index #{points_ratio_index}) quantiles #{quantiles_working_time} placed into quantile index #{quantile_index} " \ "score: #{scoring_matrix[points_ratio_index][quantile_index]}" ) @@ -217,12 +217,12 @@ exercise: matching_exercise, proxy_exercise: self, reason: @reason.to_json) ex.tags.each do |t| tags_counter[t] += 1 tag_diminishing_return_factor = tag_diminishing_return_function(tags_counter[t], all_used_tags_with_count[t]) - tag_ratio = ex.exercise_tags.where(tag: t).first.factor.to_f / ex.exercise_tags.inject(0) do |sum, et| - sum + et.factor - end - Rails.logger.debug("tag: #{t}, factor: #{ex.exercise_tags.where(tag: t).first.factor}, sumall: #{ex.exercise_tags.inject(0) do |sum, et| - sum + et.factor - end }") + tag_ratio = ex.exercise_tags.find_by(tag: t).factor.to_f / ex.exercise_tags.inject(0) do |sum, et| + sum + et.factor + end + Rails.logger.debug("tag: #{t}, factor: #{ex.exercise_tags.find_by(tag: t).factor}, sumall: #{ex.exercise_tags.inject(0) do |sum, et| + sum + et.factor + end }") Rails.logger.debug("tag #{t}, count #{tags_counter[t]}, max: #{all_used_tags_with_count[t]}, factor: #{tag_diminishing_return_factor}") Rails.logger.debug("tag_ratio #{tag_ratio}") topic_knowledge_ratio = ex.expected_difficulty * tag_ratio diff --git a/app/models/submission.rb b/app/models/submission.rb index accee1e8..ca062fa2 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -127,7 +127,7 @@ class Submission < ApplicationRecord end def own_unsolved_rfc - RequestForComment.unsolved.where(exercise_id: exercise, user_id: user_id).first + RequestForComment.unsolved.find_by(exercise_id: exercise, user_id: user_id) end def unsolved_rfc diff --git a/app/models/tag.rb b/app/models/tag.rb index d3d9c86e..a56a9363 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -6,11 +6,7 @@ class Tag < ApplicationRecord validates :name, uniqueness: true - def destroy - if can_be_destroyed? - super - end - end + before_destroy :can_be_destroyed?, prepend: true def can_be_destroyed? exercises.none? diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index c2ce6629..a54879e8 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -5,7 +5,7 @@ class ExercisePolicy < AdminOrAuthorPolicy admin? end - %i[show? feedback? statistics? get_rfcs_for_exercise?].each do |action| + %i[show? feedback? statistics? rfcs_for_exercise?].each do |action| define_method(action) { admin? || teacher_in_study_group? || teacher? && @record.public? || author? } end diff --git a/app/policies/request_for_comment_policy.rb b/app/policies/request_for_comment_policy.rb index e8a3d435..d69e0cc6 100644 --- a/app/policies/request_for_comment_policy.rb +++ b/app/policies/request_for_comment_policy.rb @@ -37,11 +37,11 @@ class RequestForCommentPolicy < ApplicationPolicy everyone end - def get_my_comment_requests? + def my_comment_requests? everyone end - def get_rfcs_with_my_comments? + def rfcs_with_my_comments? everyone end end diff --git a/app/services/exercise_service/check_external.rb b/app/services/exercise_service/check_external.rb index 2efc6f68..79836a44 100644 --- a/app/services/exercise_service/check_external.rb +++ b/app/services/exercise_service/check_external.rb @@ -3,6 +3,7 @@ module ExerciseService class CheckExternal < ServiceBase def initialize(uuid:, codeharbor_link:) + super() @uuid = uuid @codeharbor_link = codeharbor_link end diff --git a/app/services/exercise_service/push_external.rb b/app/services/exercise_service/push_external.rb index 09b777c6..69c5234a 100644 --- a/app/services/exercise_service/push_external.rb +++ b/app/services/exercise_service/push_external.rb @@ -3,6 +3,7 @@ module ExerciseService class PushExternal < ServiceBase def initialize(zip:, codeharbor_link:) + super() @zip = zip @codeharbor_link = codeharbor_link end diff --git a/app/services/proforma_service/convert_exercise_to_task.rb b/app/services/proforma_service/convert_exercise_to_task.rb index ab91b6dc..6ad3c429 100644 --- a/app/services/proforma_service/convert_exercise_to_task.rb +++ b/app/services/proforma_service/convert_exercise_to_task.rb @@ -7,6 +7,7 @@ module ProformaService DEFAULT_LANGUAGE = 'de' def initialize(exercise: nil) + super() @exercise = exercise end diff --git a/app/services/proforma_service/convert_task_to_exercise.rb b/app/services/proforma_service/convert_task_to_exercise.rb index 658b5be0..8a13f835 100644 --- a/app/services/proforma_service/convert_task_to_exercise.rb +++ b/app/services/proforma_service/convert_task_to_exercise.rb @@ -3,6 +3,7 @@ module ProformaService class ConvertTaskToExercise < ServiceBase def initialize(task:, user:, exercise: nil) + super() @task = task @user = user @exercise = exercise || Exercise.new(unpublished: true) diff --git a/app/services/proforma_service/export_task.rb b/app/services/proforma_service/export_task.rb index 72679e23..ac5ed627 100644 --- a/app/services/proforma_service/export_task.rb +++ b/app/services/proforma_service/export_task.rb @@ -3,6 +3,7 @@ module ProformaService class ExportTask < ServiceBase def initialize(exercise: nil) + super() @exercise = exercise end diff --git a/app/services/proforma_service/import.rb b/app/services/proforma_service/import.rb index e4b5fe12..2aa6b6e2 100644 --- a/app/services/proforma_service/import.rb +++ b/app/services/proforma_service/import.rb @@ -3,6 +3,7 @@ module ProformaService class Import < ServiceBase def initialize(zip:, user:) + super() @zip = zip @user = user end diff --git a/app/views/application/_session.html.slim b/app/views/application/_session.html.slim index e70e28f1..d5a1f906 100644 --- a/app/views/application/_session.html.slim +++ b/app/views/application/_session.html.slim @@ -6,8 +6,8 @@ span.caret ul.dropdown-menu.p-0.mt-1 role='menu' li = link_to(t('request_for_comments.index.all'), request_for_comments_path, class: 'dropdown-item') if policy(:request_for_comment).index? - li = link_to(t('request_for_comments.index.get_my_rfc_activity'), my_rfc_activity_path, class: 'dropdown-item') - li = link_to(t('request_for_comments.index.get_my_comment_requests'), my_request_for_comments_path, class: 'dropdown-item') + li = link_to(t('request_for_comments.index.my_rfc_activity'), my_rfc_activity_path, class: 'dropdown-item') + li = link_to(t('request_for_comments.index.my_comment_requests'), my_request_for_comments_path, class: 'dropdown-item') - if current_user.try(:admin?) or current_user.try(:teacher?) li = link_to(t('consumers.show.link'), current_user.consumer, class: 'dropdown-item') if current_user.consumer and policy(current_user.consumer).show? li = link_to(t('internal_users.show.link'), current_user, class: 'dropdown-item') if policy(current_user).show? diff --git a/app/views/exercises/index.html.slim b/app/views/exercises/index.html.slim index 60862500..46ba1d4b 100644 --- a/app/views/exercises/index.html.slim +++ b/app/views/exercises/index.html.slim @@ -45,7 +45,7 @@ h1 = Exercise.model_name.human(count: 2) ul.dropdown-menu.float-right role="menu" li = link_to(t('shared.show'), exercise, 'data-turbolinks' => "false", class: 'dropdown-item') if policy(exercise).show? li = link_to(t('activerecord.models.user_exercise_feedback.other'), feedback_exercise_path(exercise), class: 'dropdown-item') if policy(exercise).feedback? - li = link_to(t('activerecord.models.request_for_comment.other'), rfcs_for_exercise_path(exercise), class: 'dropdown-item') if policy(exercise).get_rfcs_for_exercise? + li = link_to(t('activerecord.models.request_for_comment.other'), rfcs_for_exercise_path(exercise), class: 'dropdown-item') if policy(exercise).rfcs_for_exercise? li = link_to(t('shared.destroy'), exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item') if policy(exercise).destroy? li = link_to(t('.clone'), clone_exercise_path(exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item') if policy(exercise).clone? li = link_to(t('exercises.export_codeharbor.label'), '', class: 'dropdown-item export-start', data: {'exercise-id' => exercise.id}) if policy(exercise).export_external_confirm? diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index d4e0e731..3cea7532 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -14,7 +14,7 @@ h1 li = link_to(t('exercises.index.implement'), implement_exercise_path(@exercise), 'data-turbolinks' => "false", class: 'dropdown-item text-dark') if policy(@exercise).implement? li = link_to(t('shared.statistics'), statistics_exercise_path(@exercise), 'data-turbolinks' => "false", class: 'dropdown-item text-dark') if policy(@exercise).statistics? li = link_to(t('activerecord.models.user_exercise_feedback.other'), feedback_exercise_path(@exercise), class: 'dropdown-item text-dark') if policy(@exercise).feedback? - li = link_to(t('activerecord.models.request_for_comment.other'), rfcs_for_exercise_path(@exercise), class: 'dropdown-item text-dark') if policy(@exercise).get_rfcs_for_exercise? + li = link_to(t('activerecord.models.request_for_comment.other'), rfcs_for_exercise_path(@exercise), class: 'dropdown-item text-dark') if policy(@exercise).rfcs_for_exercise? li = link_to(t('shared.destroy'), @exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete, class: 'dropdown-item text-dark') if policy(@exercise).destroy? li = link_to(t('exercises.index.clone'), clone_exercise_path(@exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post, class: 'dropdown-item text-dark') if policy(@exercise).clone? li = link_to(t('exercises.export_codeharbor.label'), '', class: 'dropdown-item export-start text-dark', data: {'exercise-id' => @exercise.id}) if policy(@exercise).export_external_confirm? diff --git a/app/views/user_mailer/got_new_comment.slim b/app/views/user_mailer/got_new_comment.slim index af647e5f..2f9977e7 100644 --- a/app/views/user_mailer/got_new_comment.slim +++ b/app/views/user_mailer/got_new_comment.slim @@ -3,5 +3,5 @@ link_to_comment: link_to(@rfc_link, @rfc_link), commenting_user_displayname: @commenting_user_displayname, comment_text: @comment_text, - link_my_comments: link_to(t('request_for_comments.index.get_my_comment_requests'), my_request_for_comments_url), + link_my_comments: link_to(t('request_for_comments.index.my_comment_requests'), my_request_for_comments_url), link_all_comments: link_to(t('request_for_comments.index.all'), request_for_comments_url) ) diff --git a/app/views/user_mailer/got_new_comment_for_subscription.html.slim b/app/views/user_mailer/got_new_comment_for_subscription.html.slim index b9e1a179..0bfae772 100644 --- a/app/views/user_mailer/got_new_comment_for_subscription.html.slim +++ b/app/views/user_mailer/got_new_comment_for_subscription.html.slim @@ -3,5 +3,5 @@ unsubscribe_link: link_to(@unsubscribe_link, @unsubscribe_link), author_displayname: @author_displayname, comment_text: @comment_text, - link_my_comments: link_to(t('request_for_comments.index.get_my_comment_requests'), my_request_for_comments_url), + link_my_comments: link_to(t('request_for_comments.index.my_comment_requests'), my_request_for_comments_url), link_all_comments: link_to(t('request_for_comments.index.all'), request_for_comments_url) ) diff --git a/config/environments/development.rb b/config/environments/development.rb index 01edd517..3a554602 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -20,7 +20,7 @@ Rails.application.configure do # Enable/disable caching. By default caching is disabled. # Run rails dev:cache to toggle caching. - if Rails.root.join('tmp', 'caching-dev.txt').exist? + if Rails.root.join('tmp/caching-dev.txt').exist? config.action_controller.perform_caching = true config.action_controller.enable_fragment_cache_logging = true diff --git a/config/initializers/monkey_patches.rb b/config/initializers/monkey_patches.rb index 5ae5de8a..201b68d5 100644 --- a/config/initializers/monkey_patches.rb +++ b/config/initializers/monkey_patches.rb @@ -3,15 +3,7 @@ unless Array.respond_to?(:average) class Array def average - inject(:+) / length if present? - end - end -end - -unless Array.respond_to?(:to_h) - class Array - def to_h - to_h + sum / length if present? end end end diff --git a/config/locales/de.yml b/config/locales/de.yml index c5b6a4f7..4c5dd2f3 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -723,11 +723,11 @@ de: Mit "Kommentar abschicken" wird der Kommentar dann gesichert und taucht als Sprechblase neben der Zeile auf. howto_title: 'Anleitung' index: - get_my_comment_requests: Meine Kommentaranfragen + my_comment_requests: Meine Kommentaranfragen all: "Alle Kommentaranfragen" - get_rfcs_with_my_comments: Kommentaranfragen die ich kommentiert habe - get_my_rfc_activity: "Meine Kommentaraktivität" - get_rfcs_for_exercise: "Übungsspezifische Kommentare" + rfcs_with_my_comments: Kommentaranfragen die ich kommentiert habe + my_rfc_activity: "Meine Kommentaraktivität" + rfcs_for_exercise: "Übungsspezifische Kommentare" study_groups: placeholder: "Lerngruppe" current: "Aktuelle Lerngruppe" diff --git a/config/locales/en.yml b/config/locales/en.yml index 528fd6a2..6fa247a6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -724,10 +724,10 @@ en: howto_title: 'How to comment' index: all: All Requests for Comments - get_my_comment_requests: My Requests for Comments - get_rfcs_with_my_comments: Requests for Comments I have commented on - get_my_rfc_activity: "My Comment Activity" - get_rfcs_for_exercise: "Exercise Comments" + my_comment_requests: My Requests for Comments + rfcs_with_my_comments: Requests for Comments I have commented on + my_rfc_activity: "My Comment Activity" + rfcs_for_exercise: "Exercise Comments" study_groups: placeholder: "Study group" current: "Current Study Group" diff --git a/config/routes.rb b/config/routes.rb index 5e512d56..244f815d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -23,9 +23,9 @@ Rails.application.routes.draw do end end resources :comments, defaults: {format: :json} - get '/my_request_for_comments', as: 'my_request_for_comments', to: 'request_for_comments#get_my_comment_requests' - get '/my_rfc_activity', as: 'my_rfc_activity', to: 'request_for_comments#get_rfcs_with_my_comments' - get '/exercises/:exercise_id/request_for_comments', as: 'rfcs_for_exercise', to: 'request_for_comments#get_rfcs_for_exercise' + get '/my_request_for_comments', as: 'my_request_for_comments', to: 'request_for_comments#my_comment_requests' + get '/my_rfc_activity', as: 'my_rfc_activity', to: 'request_for_comments#rfcs_with_my_comments' + get '/exercises/:exercise_id/request_for_comments', as: 'rfcs_for_exercise', to: 'request_for_comments#rfcs_for_exercise' delete '/comment_by_id', to: 'comments#destroy_by_id' put '/comments', to: 'comments#update', defaults: {format: :json} diff --git a/db/migrate/20150204080832_add_pool_size_to_execution_environments.rb b/db/migrate/20150204080832_add_pool_size_to_execution_environments.rb index afe35010..11b7dc79 100644 --- a/db/migrate/20150204080832_add_pool_size_to_execution_environments.rb +++ b/db/migrate/20150204080832_add_pool_size_to_execution_environments.rb @@ -6,7 +6,7 @@ class AddPoolSizeToExecutionEnvironments < ActiveRecord::Migration[4.2] reversible do |direction| direction.up do - ExecutionEnvironment.update_all(pool_size: 0) + ExecutionEnvironment.update(pool_size: 0) end end end diff --git a/db/migrate/20150317083739_add_memory_limit_to_execution_environments.rb b/db/migrate/20150317083739_add_memory_limit_to_execution_environments.rb index c9a019cc..290f6c1b 100644 --- a/db/migrate/20150317083739_add_memory_limit_to_execution_environments.rb +++ b/db/migrate/20150317083739_add_memory_limit_to_execution_environments.rb @@ -6,7 +6,7 @@ class AddMemoryLimitToExecutionEnvironments < ActiveRecord::Migration[4.2] reversible do |direction| direction.up do - ExecutionEnvironment.update_all(memory_limit: DockerClient::DEFAULT_MEMORY_LIMIT) + ExecutionEnvironment.update(memory_limit: DockerClient::DEFAULT_MEMORY_LIMIT) end end end diff --git a/db/migrate/20150317115338_add_network_enabled_to_execution_environments.rb b/db/migrate/20150317115338_add_network_enabled_to_execution_environments.rb index e5b62d0a..182cc66e 100644 --- a/db/migrate/20150317115338_add_network_enabled_to_execution_environments.rb +++ b/db/migrate/20150317115338_add_network_enabled_to_execution_environments.rb @@ -6,7 +6,7 @@ class AddNetworkEnabledToExecutionEnvironments < ActiveRecord::Migration[4.2] reversible do |direction| direction.up do - ExecutionEnvironment.update_all(network_enabled: true) + ExecutionEnvironment.update(network_enabled: true) end end end diff --git a/db/migrate/20170205210357_create_interventions.rb b/db/migrate/20170205210357_create_interventions.rb index 97f739b4..8345f895 100644 --- a/db/migrate/20170205210357_create_interventions.rb +++ b/db/migrate/20170205210357_create_interventions.rb @@ -17,6 +17,6 @@ class CreateInterventions < ActiveRecord::Migration[4.2] t.timestamps end - Intervention.createDefaultInterventions + Intervention.create_default_interventions end end diff --git a/db/migrate/20170403162848_set_default_for_request_for_comment_solved.rb b/db/migrate/20170403162848_set_default_for_request_for_comment_solved.rb index 75e88a53..bc120c46 100644 --- a/db/migrate/20170403162848_set_default_for_request_for_comment_solved.rb +++ b/db/migrate/20170403162848_set_default_for_request_for_comment_solved.rb @@ -3,6 +3,6 @@ class SetDefaultForRequestForCommentSolved < ActiveRecord::Migration[4.2] def change change_column_default :request_for_comments, :solved, false - RequestForComment.where(solved: nil).update_all(solved: false) + RequestForComment.where(solved: nil).update(solved: false) end end diff --git a/db/migrate/20170830083601_add_cause_to_testruns.rb b/db/migrate/20170830083601_add_cause_to_testruns.rb index 2fea8d54..6a5617a1 100644 --- a/db/migrate/20170830083601_add_cause_to_testruns.rb +++ b/db/migrate/20170830083601_add_cause_to_testruns.rb @@ -6,7 +6,7 @@ class AddCauseToTestruns < ActiveRecord::Migration[4.2] Testrun.reset_column_information Testrun.all.each do |testrun| if testrun.submission.nil? - say_with_time "#{testrun.id} has no submission" do end + say_with_time "#{testrun.id} has no submission" else testrun.cause = testrun.submission.cause testrun.save diff --git a/db/migrate/20180130172021_add_reached_full_score_to_request_for_comment.rb b/db/migrate/20180130172021_add_reached_full_score_to_request_for_comment.rb index ffb60187..79ecc350 100644 --- a/db/migrate/20180130172021_add_reached_full_score_to_request_for_comment.rb +++ b/db/migrate/20180130172021_add_reached_full_score_to_request_for_comment.rb @@ -4,7 +4,7 @@ class AddReachedFullScoreToRequestForComment < ActiveRecord::Migration[4.2] def up add_column :request_for_comments, :full_score_reached, :boolean, default: false RequestForComment.find_each do |rfc| - if rfc.submission.present? && rfc.submission.exercise.has_user_solved(rfc.user) + if rfc.submission.present? && rfc.submission.exercise.solved_by?(rfc.user) rfc.full_score_reached = true rfc.save end diff --git a/db/migrate/20181119161514_add_user_to_proxy_exercise.rb b/db/migrate/20181119161514_add_user_to_proxy_exercise.rb index c19ff639..59380ea3 100644 --- a/db/migrate/20181119161514_add_user_to_proxy_exercise.rb +++ b/db/migrate/20181119161514_add_user_to_proxy_exercise.rb @@ -6,6 +6,6 @@ class AddUserToProxyExercise < ActiveRecord::Migration[5.2] add_column :proxy_exercises, :public, :boolean, null: false, default: false internal_user = InternalUser.find_by(id: 46) || InternalUser.first - ProxyExercise.update_all(user_id: internal_user&.id || 1, user_type: internal_user.class.name) + ProxyExercise.update(user_id: internal_user&.id || 1, user_type: internal_user.class.name) end end diff --git a/db/migrate/20181126163428_add_user_type_to_remote_evaluation_mappings.rb b/db/migrate/20181126163428_add_user_type_to_remote_evaluation_mappings.rb index 6a1ee2ba..a3ab1a69 100644 --- a/db/migrate/20181126163428_add_user_type_to_remote_evaluation_mappings.rb +++ b/db/migrate/20181126163428_add_user_type_to_remote_evaluation_mappings.rb @@ -5,6 +5,6 @@ class AddUserTypeToRemoteEvaluationMappings < ActiveRecord::Migration[5.2] add_column :remote_evaluation_mappings, :user_type, :string # Update all existing records and set user_type to `ExternalUser` (safe way to prevent any function loss). # We are not using a default value here on intend to be in line with the other `user_type` columns - RemoteEvaluationMapping.update_all(user_type: 'ExternalUser') + RemoteEvaluationMapping.update(user_type: 'ExternalUser') end end diff --git a/db/migrate/20181129093207_drop_errors.rb b/db/migrate/20181129093207_drop_errors.rb index e959351c..76306ec9 100644 --- a/db/migrate/20181129093207_drop_errors.rb +++ b/db/migrate/20181129093207_drop_errors.rb @@ -21,7 +21,7 @@ class DropErrors < ActiveRecord::Migration[5.2] end def change - puts 'Migrating CodeOcean::Errors to StructuredErrors using RegEx. This might take a (long) while but will return.' + Rails.logger.info 'Migrating CodeOcean::Errors to StructuredErrors using RegEx. This might take a (long) while but will return.' submissions_controller = SubmissionsController.new # Iterate only over those Errors containing a message and submission_id diff --git a/db/migrate/20210512133611_add_service_name_to_active_storage_blobs.active_storage.rb b/db/migrate/20210512133611_add_service_name_to_active_storage_blobs.active_storage.rb index cdfc14f9..57d5d455 100644 --- a/db/migrate/20210512133611_add_service_name_to_active_storage_blobs.active_storage.rb +++ b/db/migrate/20210512133611_add_service_name_to_active_storage_blobs.active_storage.rb @@ -6,8 +6,8 @@ class AddServiceNameToActiveStorageBlobs < ActiveRecord::Migration[6.0] if table_exists?(:active_storage_blobs) && !column_exists?(:active_storage_blobs, :service_name) add_column :active_storage_blobs, :service_name, :string - if configured_service = ActiveStorage::Blob.service.name - ActiveStorage::Blob.unscoped.update_all(service_name: configured_service) + if (configured_service = ActiveStorage::Blob.service.name) + ActiveStorage::Blob.unscoped.update(service_name: configured_service) end change_column :active_storage_blobs, :service_name, :string, null: false diff --git a/db/seeds.rb b/db/seeds.rb index 37cfcd55..bd1e5599 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -23,7 +23,7 @@ Rails.application.eager_load! (ApplicationRecord.descendants - [ActiveRecord::SchemaMigration, User]).each(&:delete_all) # delete file uploads -FileUtils.rm_rf(Rails.root.join('public', 'uploads')) +FileUtils.rm_rf(Rails.root.join('public/uploads')) # load environment-dependent seeds -load(Rails.root.join('db', 'seeds', "#{Rails.env}.rb")) +load(Rails.root.join("db/seeds/#{Rails.env}.rb")) diff --git a/db/seeds/fibonacci/exercise.rb b/db/seeds/fibonacci/exercise.rb index e0a57e43..51af8d6a 100644 --- a/db/seeds/fibonacci/exercise.rb +++ b/db/seeds/fibonacci/exercise.rb @@ -1,3 +1,3 @@ # frozen_string_literal: true -def fibonacci(n); end +def fibonacci(number); end diff --git a/db/seeds/fibonacci/exercise_spec_3.rb b/db/seeds/fibonacci/exercise_spec_3.rb index 82f6a72b..80e5baab 100644 --- a/db/seeds/fibonacci/exercise_spec_3.rb +++ b/db/seeds/fibonacci/exercise_spec_3.rb @@ -4,11 +4,11 @@ require './exercise' require './reference' describe '#fibonacci' do - SAMPLE_COUNT = 32 + let(:sample_count) { 32 } let(:reference) { Class.new.extend(Reference) } - SAMPLE_COUNT.times do |i| + sample_count.times do |i| instance_eval do it "obtains the correct result for input #{i}" do expect(fibonacci(i)).to eq(reference.fibonacci(i)) diff --git a/db/seeds/fibonacci/reference.rb b/db/seeds/fibonacci/reference.rb index 6ee48352..2bf105ff 100644 --- a/db/seeds/fibonacci/reference.rb +++ b/db/seeds/fibonacci/reference.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Reference - def fibonacci(n) - n < 2 ? n : fibonacci(n - 1) + fibonacci(n - 2) + def fibonacci(number) + number < 2 ? number : fibonacci(number - 1) + fibonacci(number - 2) end end diff --git a/db/seeds/hello_world/exercise.rb b/db/seeds/hello_world/exercise.rb index e69de29b..bee5f768 100644 --- a/db/seeds/hello_world/exercise.rb +++ b/db/seeds/hello_world/exercise.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +# Write your code here diff --git a/db/seeds/production.rb b/db/seeds/production.rb index 2820de64..ce697318 100644 --- a/db/seeds/production.rb +++ b/db/seeds/production.rb @@ -29,7 +29,7 @@ FileType.create_factories # change all resources' author [ExecutionEnvironment, Exercise, FileType].each do |model| - model.update_all(user_id: InternalUser.first.id) + model.update(user_id: InternalUser.first.id) end # delete temporary users diff --git a/db/seeds/sql_select/comparator.rb b/db/seeds/sql_select/comparator.rb index 5b3971c8..3db50306 100644 --- a/db/seeds/sql_select/comparator.rb +++ b/db/seeds/sql_select/comparator.rb @@ -10,5 +10,7 @@ database = SQLite3::Database.new('/database.db') missing_tuples = database.execute(REFERENCE_QUERY) - database.execute(STUDENT_QUERY) unexpected_tuples = database.execute(STUDENT_QUERY) - database.execute(REFERENCE_QUERY) +# rubocop:disable Rails/Output puts("Missing tuples: #{missing_tuples}") puts("Unexpected tuples: #{unexpected_tuples}") +# rubocop:enable Rails/Output diff --git a/db/seeds/tdd/exercise.rb b/db/seeds/tdd/exercise.rb index e69de29b..bee5f768 100644 --- a/db/seeds/tdd/exercise.rb +++ b/db/seeds/tdd/exercise.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +# Write your code here diff --git a/lib/active_model/validations/boolean_presence_validator.rb b/lib/active_model/validations/boolean_presence_validator.rb index 80acaafe..fa959e6e 100644 --- a/lib/active_model/validations/boolean_presence_validator.rb +++ b/lib/active_model/validations/boolean_presence_validator.rb @@ -3,10 +3,12 @@ module ActiveModel module Validations class BooleanPresenceValidator < EachValidator + BOOLEAN_VALUES = [false, true].freeze + def validate(record) [attributes].flatten.each do |attribute| value = record.send(:read_attribute_for_validation, attribute) - record.errors.add(attribute, nil, options) unless [false, true].include?(value) + record.errors.add(attribute, nil, options) unless BOOLEAN_VALUES.include?(value) end end end diff --git a/lib/assessor.rb b/lib/assessor.rb index 47612f94..7d4bf600 100644 --- a/lib/assessor.rb +++ b/lib/assessor.rb @@ -12,7 +12,7 @@ class Assessor def calculate_score(test_outcome) score = 0.0 - if test_outcome[:passed].to_f != 0.0 && test_outcome[:count].to_f != 0.0 + if test_outcome[:passed].to_d != 0.0.to_d && test_outcome[:count].to_d != 0.0.to_d score = (test_outcome[:passed].to_f / test_outcome[:count]) # prevent negative scores score = [0.0, score].max diff --git a/lib/code_ocean/config.rb b/lib/code_ocean/config.rb index f8f186a1..e8623254 100644 --- a/lib/code_ocean/config.rb +++ b/lib/code_ocean/config.rb @@ -9,7 +9,7 @@ module CodeOcean def read(options = {}) path = Rails.root.join('config', "#{@filename}.yml#{options[:erb] ? '.erb' : ''}") if ::File.exist?(path) - content = options[:erb] ? YAML.safe_load(ERB.new(::File.new(path, 'r').read).result) : YAML.load_file(path) + content = options[:erb] ? YAML.safe_load(ERB.new(::File.new(path, 'r').read).result, aliases: true, permitted_classes: [Range]) : YAML.load_file(path) content[Rails.env].with_indifferent_access else raise Error.new("Configuration file not found: #{path}") diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 7eb4bc39..44e3eda6 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -78,7 +78,7 @@ class DockerClient } end - def create_socket(container, stderr = false) + def create_socket(container, stderr: false) # TODO: factor out query params # todo separate stderr query_params = "logs=0&stream=1&#{stderr ? 'stderr=1' : 'stdout=1&stdin=1'}" @@ -111,7 +111,7 @@ class DockerClient end def self.create_container(execution_environment) - tries ||= 0 + # tries ||= 0 # container.start sometimes creates the passed local_workspace_path on disk (depending on the setup). # this is however not guaranteed and caused issues on the server already. Therefore create the necessary folders manually! local_workspace_path = generate_local_workspace_path @@ -133,14 +133,14 @@ class DockerClient Thread.new do timeout = SELF_DESTROY_GRACE_PERIOD.to_i sleep(timeout) - container.docker_client.kill_container(container, false) + container.docker_client.kill_container(container) Rails.logger.info("Force killing container in status #{container.status} after #{Time.zone.now - container.start_time} seconds.") ensure # guarantee that the thread is releasing the DB connection after it is done ActiveRecord::Base.connection_pool.release_connection end else - container.docker_client.kill_container(container, false) + container.docker_client.kill_container(container) Rails.logger.info("Killing container in status #{container.status} after #{Time.zone.now - container.start_time} seconds.") end ensure @@ -280,7 +280,7 @@ class DockerClient {status: :container_depleted, waiting_for_container_time: waiting_for_container_time, container_execution_time: nil} end - rescue Excon::Errors::SocketError => e + rescue Excon::Errors::SocketError # socket errors seems to be normal when using exec # so lets ignore them for now # (tries += 1) <= RETRY_COUNT ? retry : raise(error) @@ -310,10 +310,8 @@ container_execution_time: nil} end def kill_after_timeout(container) - " - We need to start a second thread to kill the websocket connection, - as it is impossible to determine whether further input is requested. - " + # We need to start a second thread to kill the websocket connection, + # as it is impossible to determine whether further input is requested. container.status = :executing @thread = Thread.new do timeout = @execution_environment.permitted_execution_time.to_i # seconds @@ -365,16 +363,14 @@ container_execution_time: nil} end end - def kill_container(container, _create_new = true) + def kill_container(container) exit_thread_if_alive Rails.logger.info("killing container #{container}") self.class.destroy_container(container) end def execute_run_command(submission, filename, &block) - " - Run commands by attaching a websocket to Docker. - " + # Run commands by attaching a websocket to Docker. filepath = submission.collect_files.find {|f| f.name_with_extension == filename }.filepath command = submission.execution_environment.run_command % command_substitutions(filepath) create_workspace_files = proc { create_workspace_files(container, submission) } @@ -383,9 +379,7 @@ container_execution_time: nil} end def execute_test_command(submission, filename, &block) - " - Stick to existing Docker API with exec command. - " + # Stick to existing Docker API with exec command. file = submission.collect_files.find {|f| f.name_with_extension == filename } filepath = file.filepath command = submission.execution_environment.test_command % command_substitutions(filepath) @@ -495,26 +489,23 @@ container_execution_time: nil} if output.nil? kill_container(container) else - result = {status: (output[2]).zero? ? :ok : :failed, stdout: output[0].join.force_encoding('utf-8'), -stderr: output[1].join.force_encoding('utf-8')} + result = {status: (output[2])&.zero? ? :ok : :failed, stdout: output[0].join.force_encoding('utf-8'), stderr: output[1].join.force_encoding('utf-8')} end # if we use pooling and recylce the containers, put it back. otherwise, destroy it. if DockerContainerPool.config[:active] && RECYCLE_CONTAINERS - self.class.return_container(container, - @execution_environment) + self.class.return_container(container, @execution_environment) else self.class.destroy_container(container) end result rescue Timeout::Error Rails.logger.info("got timeout error for container #{container}") - stdout = container.exec(['cat', '/tmp/stdout.log'])[0].join.force_encoding('utf-8') - stderr = container.exec(['cat', '/tmp/stderr.log'])[0].join.force_encoding('utf-8') + stdout = container.exec(%w[cat /tmp/stdout.log])[0].join.force_encoding('utf-8') + stderr = container.exec(%w[cat /tmp/stderr.log])[0].join.force_encoding('utf-8') kill_container(container) {status: :timeout, stdout: stdout, stderr: stderr} end - private :send_command class Error < RuntimeError; end diff --git a/lib/file_tree.rb b/lib/file_tree.rb index 672ceab8..bfcf766a 100644 --- a/lib/file_tree.rb +++ b/lib/file_tree.rb @@ -52,9 +52,7 @@ class FileTree < Tree::TreeNode private :map_to_js_tree def node_icon(node) - if node.is_root? - folder_icon - elsif node.is_leaf? + if node.is_leaf? && !node.is_root? file_icon(node.content) else folder_icon diff --git a/lib/python20_course_week.rb b/lib/python20_course_week.rb index 0d4d9bc7..4b7e4f79 100644 --- a/lib/python20_course_week.rb +++ b/lib/python20_course_week.rb @@ -9,13 +9,9 @@ class Python20CourseWeek 2 when /Python20 Aufgabe 3/ 3 - when /Python20 Aufgabe 4/ + when /Python20 Aufgabe 4/, /Python20 Snake/ 4 - when /Python20 Snake/ - 4 - else - # Not part of the Python20 course - nil + # else: Not part of the Python20 course end end diff --git a/lib/tasks/detect_exercise_anomalies.rake b/lib/tasks/detect_exercise_anomalies.rake index cb0ca5f1..203f7dfc 100644 --- a/lib/tasks/detect_exercise_anomalies.rake +++ b/lib/tasks/detect_exercise_anomalies.rake @@ -81,7 +81,7 @@ namespace :detect_exercise_anomalies do def find_anomalies(collection) working_times = collect_working_times(collection).compact if working_times.values.size.positive? - average = working_times.values.reduce(:+) / working_times.values.size + average = working_times.values.sum / working_times.values.size return working_times.select do |_, working_time| working_time > average * MAX_TIME_FACTOR or working_time < average * MIN_TIME_FACTOR end @@ -120,7 +120,8 @@ namespace :detect_exercise_anomalies do users_to_notify = [] users = {} - %i[performers_by_time performers_by_score].each do |method| + methods = %i[performers_by_time performers_by_score] + methods.each do |method| # merge users found by multiple methods returning a hash {best: [], worst: []} users = users.merge(send(method, exercise, NUMBER_OF_USERS_PER_CLASS)) {|_key, this, other| this + other } end diff --git a/spec/concerns/lti_spec.rb b/spec/concerns/lti_spec.rb index beff3298..71ac0bd9 100644 --- a/spec/concerns/lti_spec.rb +++ b/spec/concerns/lti_spec.rb @@ -33,11 +33,11 @@ describe Lti do let(:last_name) { 'Doe' } let(:full_name) { 'John Doe' } let(:provider) { double } - let(:provider_full) { double(lis_person_name_full: full_name) } + let(:provider_full) { instance_double('IMS::LTI::ToolProvider', lis_person_name_full: full_name) } context 'when a full name is provided' do it 'returns the full name' do - expect(provider_full).to receive(:lis_person_name_full).twice.and_return(full_name) + allow(provider_full).to receive(:lis_person_name_full).and_return(full_name) expect(controller.send(:external_user_name, provider_full)).to eq(full_name) end end @@ -45,7 +45,7 @@ describe Lti do context 'when only partial information is provided' do it 'returns the first available name' do expect(provider).to receive(:lis_person_name_full) - expect(provider).to receive(:lis_person_name_given).and_return(first_name) + allow(provider).to receive(:lis_person_name_given).and_return(first_name) expect(provider).not_to receive(:lis_person_name_family) expect(controller.send(:external_user_name, provider)).to eq(first_name) end @@ -64,7 +64,7 @@ describe Lti do context 'with a return URL' do let(:consumer_return_url) { 'http://example.org' } - before { expect(controller).to receive(:params).and_return(launch_presentation_return_url: consumer_return_url) } + before { allow(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) @@ -80,8 +80,8 @@ describe Lti do context 'without a return URL' do before do - expect(controller).to receive(:params).and_return({}) - expect(controller).to receive(:redirect_to).with(:root) + allow(controller).to receive(:params).and_return({}) + allow(controller).to receive(:redirect_to).with(:root) end it 'redirects to the root URL' do @@ -104,7 +104,10 @@ describe Lti do let(:consumer) { FactoryBot.create(:consumer) } let(:score) { 0.5 } let(:submission) { FactoryBot.create(:submission) } - let!(:lti_parameter) { FactoryBot.create(:lti_parameter, consumers_id: consumer.id, external_users_id: submission.user_id, exercises_id: submission.exercise_id) } + + before do + FactoryBot.create(:lti_parameter, consumers_id: consumer.id, external_users_id: submission.user_id, exercises_id: submission.exercise_id) + end context 'with an invalid score' do it 'raises an exception' do @@ -117,7 +120,7 @@ describe Lti do context 'with a tool consumer' do context 'when grading is not supported' do it 'returns a corresponding status' do - expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:outcome_service?).and_return(false) + allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:outcome_service?).and_return(false) allow(submission).to receive(:normalized_score).and_return score expect(controller.send(:send_score, submission)[:status]).to eq('unsupported') end @@ -127,12 +130,12 @@ describe Lti do let(:response) { double } before do - expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:outcome_service?).and_return(true) - expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:post_replace_result!).with(score).and_return(response) - expect(response).to receive(:response_code).at_least(:once).and_return(200) - expect(response).to receive(:post_response).and_return(response) - expect(response).to receive(:body).at_least(:once).and_return('') - expect(response).to receive(:code_major).at_least(:once).and_return('success') + allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:outcome_service?).and_return(true) + allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:post_replace_result!).with(score).and_return(response) + allow(response).to receive(:response_code).at_least(:once).and_return(200) + allow(response).to receive(:post_response).and_return(response) + allow(response).to receive(:body).at_least(:once).and_return('') + allow(response).to receive(:code_major).at_least(:once).and_return('success') end it 'sends the score' do diff --git a/spec/concerns/submission_scoring_spec.rb b/spec/concerns/submission_scoring_spec.rb index 59b08312..3a8ec5e7 100644 --- a/spec/concerns/submission_scoring_spec.rb +++ b/spec/concerns/submission_scoring_spec.rb @@ -8,30 +8,29 @@ end describe SubmissionScoring do let(:controller) { Controller.new } - - before(:all) { @submission = FactoryBot.create(:submission, cause: 'submit') } + let(:submission) { FactoryBot.create(:submission, cause: 'submit') } before { controller.instance_variable_set(:@current_user, FactoryBot.create(:external_user)) } describe '#collect_test_results' do - after { controller.send(:collect_test_results, @submission) } + after { controller.send(:collect_test_results, submission) } it 'executes every teacher-defined test file' do - @submission.collect_files.select(&:teacher_defined_assessment?).each do |file| - expect(controller).to receive(:execute_test_file).with(file, @submission).and_return({}) + submission.collect_files.select(&:teacher_defined_assessment?).each do |file| + allow(controller).to receive(:execute_test_file).with(file, submission).and_return({}) end end end describe '#score_submission' do - after { controller.score_submission(@submission) } + after { controller.score_submission(submission) } it 'collects the test results' do - expect(controller).to receive(:collect_test_results).and_return([]) + allow(controller).to receive(:collect_test_results).and_return([]) end it 'assigns a score to the submissions' do - expect(@submission).to receive(:update).with(score: anything) + expect(submission).to receive(:update).with(score: anything) end end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 541c95a5..014b6266 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -27,7 +27,7 @@ describe ApplicationController do describe '#render_not_authorized' do before do - expect(controller).to receive(:welcome) { controller.send(:render_not_authorized) } + allow(controller).to receive(:welcome) { controller.send(:render_not_authorized) } get :welcome end @@ -41,14 +41,14 @@ describe ApplicationController do context 'when specifying a locale' do before { allow(session).to receive(:[]=).at_least(:once) } - context "using the 'custom_locale' parameter" do + context "when using the 'custom_locale' parameter" do it 'overwrites the session' do expect(session).to receive(:[]=).with(:locale, locale.to_s) get :welcome, params: {custom_locale: locale} end end - context "using the 'locale' parameter" do + context "when using the 'locale' parameter" do it 'overwrites the session' do expect(session).to receive(:[]=).with(:locale, locale.to_s) get :welcome, params: {locale: locale} diff --git a/spec/controllers/codeharbor_links_controller_spec.rb b/spec/controllers/codeharbor_links_controller_spec.rb index 41363805..4b073fe7 100644 --- a/spec/controllers/codeharbor_links_controller_spec.rb +++ b/spec/controllers/codeharbor_links_controller_spec.rb @@ -14,7 +14,7 @@ describe CodeharborLinksController do allow(controller).to receive(:current_user).and_return(user) end - context 'GET #new' do + describe 'GET #new' do before do get :new end diff --git a/spec/controllers/consumers_controller_spec.rb b/spec/controllers/consumers_controller_spec.rb index 209503a5..6868ba96 100644 --- a/spec/controllers/consumers_controller_spec.rb +++ b/spec/controllers/consumers_controller_spec.rb @@ -54,9 +54,10 @@ describe ConsumersController do end describe 'GET #index' do - let!(:consumers) { FactoryBot.create_pair(:consumer) } - - before { get :index } + before do + FactoryBot.create_pair(:consumer) + get :index + end expect_assigns(consumers: Consumer.all) expect_status(200) diff --git a/spec/controllers/execution_environments_controller_spec.rb b/spec/controllers/execution_environments_controller_spec.rb index 6dc8a95f..6736a18b 100644 --- a/spec/controllers/execution_environments_controller_spec.rb +++ b/spec/controllers/execution_environments_controller_spec.rb @@ -9,7 +9,7 @@ describe ExecutionEnvironmentsController do before { allow(controller).to receive(:current_user).and_return(user) } describe 'POST #create' do - before { expect(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) } + before { allow(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) } context 'with a valid execution environment' do let(:perform_request) { proc { post :create, params: {execution_environment: FactoryBot.build(:ruby).attributes} } } @@ -50,7 +50,7 @@ describe ExecutionEnvironmentsController do describe 'GET #edit' do before do - expect(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) + allow(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) get :edit, params: {id: execution_environment.id} end @@ -64,8 +64,8 @@ describe ExecutionEnvironmentsController do let(:command) { 'which ruby' } before do - 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) + allow(DockerClient).to receive(:new).with(execution_environment: execution_environment).and_call_original + allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).with(command) post :execute_command, params: {command: command, id: execution_environment.id} end @@ -76,9 +76,10 @@ describe ExecutionEnvironmentsController do end describe 'GET #index' do - before(:all) { FactoryBot.create_pair(:ruby) } - - before { get :index } + before do + FactoryBot.create_pair(:ruby) + get :index + end expect_assigns(execution_environments: ExecutionEnvironment.all) expect_status(200) @@ -87,7 +88,7 @@ describe ExecutionEnvironmentsController do describe 'GET #new' do before do - expect(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) + allow(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) get :new end @@ -102,8 +103,8 @@ describe ExecutionEnvironmentsController do let(:docker_images) { [1, 2, 3] } before do - expect(DockerClient).to receive(:check_availability!).at_least(:once) - expect(DockerClient).to receive(:image_tags).and_return(docker_images) + allow(DockerClient).to receive(:check_availability!).at_least(:once) + allow(DockerClient).to receive(:image_tags).and_return(docker_images) controller.send(:set_docker_images) end @@ -114,7 +115,7 @@ describe ExecutionEnvironmentsController do let(:error_message) { 'Docker is unavailable' } before do - expect(DockerClient).to receive(:check_availability!).at_least(:once).and_raise(DockerClient::Error.new(error_message)) + allow(DockerClient).to receive(:check_availability!).at_least(:once).and_raise(DockerClient::Error.new(error_message)) controller.send(:set_docker_images) end @@ -154,7 +155,7 @@ describe ExecutionEnvironmentsController do describe 'PUT #update' do context 'with a valid execution environment' do before do - expect(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) + allow(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) put :update, params: {execution_environment: FactoryBot.attributes_for(:ruby), id: execution_environment.id} end diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index fc30eb21..0332641f 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -45,7 +45,7 @@ describe ExercisesController do context 'when saving fails' do before do - expect_any_instance_of(Exercise).to receive(:save).and_return(false) + allow_any_instance_of(Exercise).to receive(:save).and_return(false) perform_request.call end @@ -87,7 +87,7 @@ describe ExercisesController do let(:files_attributes) { {'0' => FactoryBot.build(:file, file_type: file_type).attributes.merge(content: uploaded_file)} } context 'when uploading a binary file' do - let(:file_path) { Rails.root.join('db', 'seeds', 'audio_video', 'devstories.mp4') } + let(:file_path) { Rails.root.join('db/seeds/audio_video/devstories.mp4') } let(:file_type) { FactoryBot.create(:dot_mp4) } let(:uploaded_file) { Rack::Test::UploadedFile.new(file_path, 'video/mp4', true) } @@ -102,7 +102,7 @@ describe ExercisesController do end context 'when uploading a non-binary file' do - let(:file_path) { Rails.root.join('db', 'seeds', 'fibonacci', 'exercise.rb') } + let(:file_path) { Rails.root.join('db/seeds/fibonacci/exercise.rb') } let(:file_type) { FactoryBot.create(:dot_rb) } let(:uploaded_file) { Rack::Test::UploadedFile.new(file_path, 'text/x-ruby', false) } @@ -189,9 +189,10 @@ describe ExercisesController do describe 'GET #index' do let(:scope) { Pundit.policy_scope!(user, Exercise) } - before(:all) { FactoryBot.create_pair(:dummy) } - - before { get :index } + before do + FactoryBot.create_pair(:dummy) + get :index + end expect_assigns(exercises: :scope) expect_status(200) @@ -208,7 +209,7 @@ describe ExercisesController do end describe 'GET #show' do - context 'as admin' do + context 'when being admin' do before { get :show, params: {id: exercise.id} } expect_assigns(exercise: :exercise) @@ -218,7 +219,7 @@ describe ExercisesController do end describe 'GET #reload' do - context 'as anyone' do + context 'when being anyone' do before { get :reload, format: :json, params: {id: exercise.id} } expect_assigns(exercise: :exercise) @@ -239,22 +240,22 @@ describe ExercisesController do let(:output) { {} } let(:perform_request) { post :submit, format: :json, params: {id: exercise.id, submission: {cause: 'submit', exercise_id: exercise.id}} } let(:user) { FactoryBot.create(:external_user) } - let!(:lti_parameter) { FactoryBot.create(:lti_parameter, external_user: user, exercise: exercise) } before do + FactoryBot.create(:lti_parameter, external_user: user, exercise: exercise) allow_any_instance_of(Submission).to receive(:normalized_score).and_return(1) - expect(controller).to receive(:collect_test_results).and_return([{score: 1, weight: 1}]) - expect(controller).to receive(:score_submission).and_call_original + allow(controller).to receive(:collect_test_results).and_return([{score: 1, weight: 1}]) + allow(controller).to receive(:score_submission).and_call_original end context 'when LTI outcomes are supported' do before do - expect(controller).to receive(:lti_outcome_service?).and_return(true) + allow(controller).to receive(:lti_outcome_service?).and_return(true) end context 'when the score transmission succeeds' do before do - expect(controller).to receive(:send_score).and_return(status: 'success') + allow(controller).to receive(:send_score).and_return(status: 'success') perform_request end @@ -270,7 +271,7 @@ describe ExercisesController do context 'when the score transmission fails' do before do - expect(controller).to receive(:send_score).and_return(status: 'unsupported') + allow(controller).to receive(:send_score).and_return(status: 'unsupported') perform_request end @@ -287,8 +288,7 @@ describe ExercisesController do context 'when LTI outcomes are not supported' do before do - expect(controller).to receive(:lti_outcome_service?).and_return(false) - expect(controller).not_to receive(:send_score) + allow(controller).to receive(:lti_outcome_service?).and_return(false) perform_request end @@ -298,6 +298,10 @@ describe ExercisesController do expect(assigns(:submission)).to be_a(Submission) end + it 'does not send scores' do + expect(controller).not_to receive(:send_score) + end + expect_json expect_status(200) end @@ -333,7 +337,7 @@ describe ExercisesController do let(:external_check_hash) { {message: message, exercise_found: true, update_right: update_right, error: error} } let(:message) { 'message' } let(:update_right) { true } - let(:error) {} + let(:error) { nil } before { allow(ExerciseService::CheckExternal).to receive(:call).with(uuid: exercise.uuid, codeharbor_link: codeharbor_link).and_return(external_check_hash) } @@ -384,7 +388,7 @@ describe ExercisesController do let!(:codeharbor_link) { FactoryBot.create(:codeharbor_link, user: user) } let(:post_request) { post :export_external_confirm, params: {id: exercise.id, codeharbor_link: codeharbor_link.id} } - let(:error) {} + let(:error) { nil } let(:zip) { 'zip' } before do diff --git a/spec/controllers/file_types_controller_spec.rb b/spec/controllers/file_types_controller_spec.rb index 783b3599..5035ceb8 100644 --- a/spec/controllers/file_types_controller_spec.rb +++ b/spec/controllers/file_types_controller_spec.rb @@ -57,9 +57,10 @@ describe FileTypesController do end describe 'GET #index' do - before(:all) { FactoryBot.create_pair(:dot_rb) } - - before { get :index } + before do + FactoryBot.create_pair(:dot_rb) + get :index + end expect_assigns(file_types: FileType.all) expect_status(200) diff --git a/spec/controllers/internal_users_controller_spec.rb b/spec/controllers/internal_users_controller_spec.rb index 5b12e492..c6a1cb9a 100644 --- a/spec/controllers/internal_users_controller_spec.rb +++ b/spec/controllers/internal_users_controller_spec.rb @@ -45,6 +45,9 @@ describe InternalUsersController do before do user.send(:setup_activation) user.save(validate: false) + end + + it 'adds an activation token' do expect(user.activation_token).to be_present end @@ -171,7 +174,7 @@ describe InternalUsersController do before do allow(controller).to receive(:set_sentry_context).and_return(nil) - expect(controller).to receive(:current_user).and_return(nil) + allow(controller).to receive(:current_user).and_return(nil) get :forgot_password end @@ -183,7 +186,7 @@ describe InternalUsersController do before do allow(controller).to receive(:set_sentry_context).and_return(nil) - expect(controller).to receive(:current_user).and_return(user) + allow(controller).to receive(:current_user).and_return(user) get :forgot_password end @@ -199,7 +202,7 @@ describe InternalUsersController do before { perform_request.call } it 'delivers instructions to reset the password' do - expect(InternalUser).to receive(:where).and_return([user]) + allow(InternalUser).to receive(:where).and_return([user]) expect(user).to receive(:deliver_reset_password_instructions!) perform_request.call end diff --git a/spec/controllers/request_for_comments_controller_spec.rb b/spec/controllers/request_for_comments_controller_spec.rb index 074aa644..1556af9a 100644 --- a/spec/controllers/request_for_comments_controller_spec.rb +++ b/spec/controllers/request_for_comments_controller_spec.rb @@ -32,24 +32,24 @@ describe RequestForCommentsController do end end - describe 'GET #get_my_comment_requests' do - before { get :get_my_comment_requests } + describe 'GET #my_comment_requests' do + before { get :my_comment_requests } expect_status(200) expect_template(:index) end - describe 'GET #get_rfcs_with_my_comments' do - before { get :get_rfcs_with_my_comments } + describe 'GET #rfcs_with_my_comments' do + before { get :rfcs_with_my_comments } expect_status(200) expect_template(:index) end - describe 'GET #get_rfcs_for_exercise' do + describe 'GET #rfcs_for_exercise' do before do exercise = FactoryBot.create(:even_odd) - get :get_rfcs_for_exercise, params: {exercise_id: exercise.id} + get :rfcs_for_exercise, params: {exercise_id: exercise.id} end expect_status(200) diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 9d882c4a..46265fb7 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -58,8 +58,8 @@ describe SessionsController do context 'without a unique OAuth nonce' do it 'refuses the LTI launch' do - expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) - expect(NonceStore).to receive(:has?).with(nonce).and_return(true) + allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) + allow(NonceStore).to receive(:has?).with(nonce).and_return(true) expect(controller).to receive(:refuse_lti_launch).with(message: I18n.t('sessions.oauth.used_nonce')).and_call_original post :create_through_lti, params: {oauth_consumer_key: consumer.oauth_key, oauth_nonce: nonce, oauth_signature: SecureRandom.hex} end @@ -67,7 +67,7 @@ describe SessionsController do context 'without a valid exercise token' do it 'refuses the LTI launch' do - expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) + allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) expect(controller).to receive(:refuse_lti_launch).with(message: I18n.t('sessions.oauth.invalid_exercise_token')).and_call_original post :create_through_lti, params: {custom_token: '', oauth_consumer_key: consumer.oauth_key, oauth_nonce: nonce, oauth_signature: SecureRandom.hex, user_id: '123'} end @@ -78,7 +78,7 @@ describe SessionsController do let(:perform_request) { post :create_through_lti, params: {custom_locale: locale, custom_token: exercise.token, oauth_consumer_key: consumer.oauth_key, oauth_nonce: nonce, oauth_signature: SecureRandom.hex, user_id: user.external_id} } let(:user) { FactoryBot.create(:external_user, consumer_id: consumer.id) } - before { expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) } + before { allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) } it 'assigns the current user' do perform_request @@ -112,7 +112,7 @@ describe SessionsController do let(:message) { I18n.t('sessions.create_through_lti.session_with_outcome', consumer: consumer) } before do - expect(controller).to receive(:lti_outcome_service?).and_return(true) + allow(controller).to receive(:lti_outcome_service?).and_return(true) perform_request end @@ -123,7 +123,7 @@ describe SessionsController do let(:message) { I18n.t('sessions.create_through_lti.session_without_outcome', consumer: consumer) } before do - expect(controller).to receive(:lti_outcome_service?).and_return(false) + allow(controller).to receive(:lti_outcome_service?).and_return(false) perform_request end @@ -159,7 +159,7 @@ describe SessionsController do before do allow(controller).to receive(:set_sentry_context).and_return(nil) - expect(controller).to receive(:current_user).at_least(:once).and_return(user) + allow(controller).to receive(:current_user).at_least(:once).and_return(user) end context 'with an internal user' do @@ -199,7 +199,14 @@ describe SessionsController do describe 'GET #destroy_through_lti' do let(:perform_request) { proc { get :destroy_through_lti, params: {consumer_id: consumer.id, submission_id: submission.id} } } - let(:submission) { FactoryBot.create(:submission, exercise: FactoryBot.create(:dummy)) } before { perform_request.call } + let(:submission) { FactoryBot.create(:submission, exercise: FactoryBot.create(:dummy)) } + + before do + # Todo replace session with lti_parameter + # Todo create LtiParameter Object + # session[:lti_parameters] = {} + perform_request.call + end it 'clears the session' do # Todo replace session with lti_parameter /should be done already @@ -216,7 +223,7 @@ describe SessionsController do before do allow(controller).to receive(:set_sentry_context).and_return(nil) - expect(controller).to receive(:current_user).and_return(nil) + allow(controller).to receive(:current_user).and_return(nil) get :new end @@ -228,7 +235,7 @@ describe SessionsController do before do allow(controller).to receive(:set_sentry_context).and_return(nil) - expect(controller).to receive(:current_user).and_return(FactoryBot.build(:teacher)) + allow(controller).to receive(:current_user).and_return(FactoryBot.build(:teacher)) get :new end diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index 448c8bf6..61e21c98 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -50,7 +50,7 @@ describe SubmissionsController do before { get :download_file, params: {filename: file.name_with_extension, id: submission.id} } - context 'for a binary file' do + context 'with a binary file' do let(:file) { submission.collect_files.detect {|file| file.name == 'exercise' && file.file_type.file_extension == '.sql' } } expect_assigns(file: :file) @@ -69,7 +69,7 @@ describe SubmissionsController do before { get :download_file, params: {filename: file.name_with_extension, id: submission.id} } - context 'for a binary file' do + context 'with a binary file' do let(:file) { submission.collect_files.detect {|file| file.file_type.file_extension == '.mp4' } } expect_assigns(file: :file) @@ -82,7 +82,7 @@ describe SubmissionsController do end end - context 'for a non-binary file' do + context 'with a non-binary file' do let(:file) { submission.collect_files.detect {|file| file.file_type.file_extension == '.js' } } expect_assigns(file: :file) @@ -98,9 +98,10 @@ describe SubmissionsController do end describe 'GET #index' do - before(:all) { FactoryBot.create_pair(:submission) } - - before { get :index } + before do + FactoryBot.create_pair(:submission) + get :index + end expect_assigns(submissions: Submission.all) expect_status(200) @@ -121,7 +122,7 @@ describe SubmissionsController do before { get :render_file, params: {filename: file.name_with_extension, id: submission.id} } - context 'for a binary file' do + context 'with a binary file' do let(:file) { submission.collect_files.detect {|file| file.file_type.file_extension == '.mp4' } } expect_assigns(file: :file) @@ -134,7 +135,7 @@ describe SubmissionsController do end end - context 'for a non-binary file' do + context 'with a non-binary file' do let(:file) { submission.collect_files.detect {|file| file.file_type.file_extension == '.js' } } expect_assigns(file: :file) @@ -154,12 +155,12 @@ describe SubmissionsController do let(:perform_request) { get :run, params: {filename: filename, id: submission.id} } before do - expect_any_instance_of(ActionController::Live::SSE).to receive(:write).at_least(3).times + allow_any_instance_of(ActionController::Live::SSE).to receive(:write).at_least(3).times end context 'when no errors occur during execution' do before do - expect_any_instance_of(DockerClient).to receive(:execute_run_command).with(submission, filename).and_return({}) + allow_any_instance_of(DockerClient).to receive(:execute_run_command).with(submission, filename).and_return({}) perform_request end @@ -222,7 +223,7 @@ describe SubmissionsController do let(:output) { {} } before do - expect_any_instance_of(DockerClient).to receive(:execute_test_command).with(submission, filename) + allow_any_instance_of(DockerClient).to receive(:execute_test_command).with(submission, filename) get :test, params: {filename: filename, id: submission.id} end diff --git a/spec/db/seeds_spec.rb b/spec/db/seeds_spec.rb index f44b6b7c..4e3944bc 100644 --- a/spec/db/seeds_spec.rb +++ b/spec/db/seeds_spec.rb @@ -9,7 +9,9 @@ describe 'seeds' do CodeOcean::Application.load_tasks # We want to execute the seeds for the dev environment against the test database + # rubocop:disable Rails/Inquiry allow(Rails).to receive(:env) { 'development'.inquiry } + # rubocop:enable Rails/Inquiry allow(ActiveRecord::Base).to receive(:establish_connection).and_call_original allow(ActiveRecord::Base).to receive(:establish_connection).with(:development) { ActiveRecord::Base.establish_connection(:test) diff --git a/spec/factories/code_ocean/file.rb b/spec/factories/code_ocean/file.rb index 3e3132ee..6b173268 100644 --- a/spec/factories/code_ocean/file.rb +++ b/spec/factories/code_ocean/file.rb @@ -16,7 +16,7 @@ module CodeOcean trait(:image) do association :file_type, factory: :dot_png name { 'poster' } - native_file { Rack::Test::UploadedFile.new(Rails.root.join('db', 'seeds', 'audio_video', 'poster.png'), 'image/png') } + native_file { Rack::Test::UploadedFile.new(Rails.root.join('db/seeds/audio_video/poster.png'), 'image/png') } end end diff --git a/spec/factories/lti_parameter.rb b/spec/factories/lti_parameter.rb index 6d5044e1..d95988cc 100644 --- a/spec/factories/lti_parameter.rb +++ b/spec/factories/lti_parameter.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true FactoryBot.define do - LTI_PARAMETERS = { + lti_params = { lis_result_sourcedid: 'c2db0c7c-4411-4b27-a52b-ddfc3dc32065', lis_outcome_service_url: 'http://172.16.54.235:3000/courses/0132156a-9afb-434d-83cc-704780104105/sections/21c6c6f4-1fb6-43b4-af3c-04fdc098879e/items/999b1fe6-d4b6-47b7-a577-ea2b4b1041ec/tool_grading', launch_presentation_return_url: 'http://172.16.54.235:3000/courses/0132156a-9afb-434d-83cc-704780104105/sections/21c6c6f4-1fb6-43b4-af3c-04fdc098879e/items/999b1fe6-d4b6-47b7-a577-ea2b4b1041ec/tool_return', @@ -12,10 +12,10 @@ FactoryBot.define do association :exercise, factory: :math association :external_user - lti_parameters { LTI_PARAMETERS } + lti_parameters { lti_params } trait :without_outcome_service_url do - lti_parameters { LTI_PARAMETERS.except(:lis_outcome_service_url) } + lti_parameters { lti_params.except(:lis_outcome_service_url) } end end end diff --git a/spec/features/authorization_spec.rb b/spec/features/authorization_spec.rb index 4fedf09e..8200964e 100644 --- a/spec/features/authorization_spec.rb +++ b/spec/features/authorization_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' describe 'Authorization' do - context 'as an admin' do + context 'when being an admin' do let(:user) { FactoryBot.create(:admin) } before { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } @@ -13,7 +13,7 @@ describe 'Authorization' do end end - context 'as an external user' do + context 'with being an external user' do let(:user) { FactoryBot.create(:external_user) } before { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } @@ -23,7 +23,7 @@ describe 'Authorization' do end end - context 'as a teacher' do + context 'with being a teacher' do let(:user) { FactoryBot.create(:teacher) } before { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } diff --git a/spec/features/editor_spec.rb b/spec/features/editor_spec.rb index 74436cdc..334cc7f0 100644 --- a/spec/features/editor_spec.rb +++ b/spec/features/editor_spec.rb @@ -29,7 +29,7 @@ describe 'Editor', js: true do fill_in('email', with: user.email) fill_in('password', with: FactoryBot.attributes_for(:teacher)[:password]) click_button(I18n.t('sessions.new.link')) - expect_any_instance_of(LtiHelper).to receive(:lti_outcome_service?).and_return(true) + allow_any_instance_of(LtiHelper).to receive(:lti_outcome_service?).and_return(true) visit(implement_exercise_path(exercise)) end diff --git a/spec/features/prometheus/controller_spec.rb b/spec/features/prometheus/controller_spec.rb index 7aebaeaa..156e227c 100644 --- a/spec/features/prometheus/controller_spec.rb +++ b/spec/features/prometheus/controller_spec.rb @@ -7,8 +7,9 @@ describe Prometheus::Controller do let(:prometheus_config) { {prometheus_exporter: {enabled: true}} } def stub_metrics + metrics = %i[@instance_count @rfc_count @rfc_commented_count] %i[increment decrement observe].each do |method| - %i[@instance_count @rfc_count @rfc_commented_count].each do |metric| + metrics.each do |metric| allow(described_class.instance_variable_get(metric)).to receive(method) end end diff --git a/spec/lib/assessor_spec.rb b/spec/lib/assessor_spec.rb index bc340536..457a6cba 100644 --- a/spec/lib/assessor_spec.rb +++ b/spec/lib/assessor_spec.rb @@ -11,7 +11,7 @@ describe Assessor do context 'when an error occurs' do before do - expect_any_instance_of(TestingFrameworkAdapter).to receive(:test_outcome).and_raise + allow_any_instance_of(TestingFrameworkAdapter).to receive(:test_outcome).and_raise end it 'catches the error' do diff --git a/spec/lib/code_ocean/config_spec.rb b/spec/lib/code_ocean/config_spec.rb index 3309b2fb..0fb183dd 100644 --- a/spec/lib/code_ocean/config_spec.rb +++ b/spec/lib/code_ocean/config_spec.rb @@ -4,7 +4,7 @@ require 'rails_helper' describe CodeOcean::Config do describe '#read' do - let(:content) { {foo: 'bar'} } + let(:content) { {'foo' => 'bar'} } let(:filename) { :foo } context 'with a .yml file' do diff --git a/spec/lib/docker_client_spec.rb b/spec/lib/docker_client_spec.rb index 609c0c69..1a2eda42 100644 --- a/spec/lib/docker_client_spec.rb +++ b/spec/lib/docker_client_spec.rb @@ -3,8 +3,6 @@ require 'rails_helper' require 'seeds_helper' -# rubocop:disable RSpec/MultipleMemoizedHelpers - WORKSPACE_PATH = Rails.root.join('tmp', 'files', Rails.env, 'code_ocean_test') describe DockerClient, docker: true do @@ -26,14 +24,14 @@ describe DockerClient, docker: true do describe '.check_availability!' do context 'when a socket error occurs' do it 'raises an error' do - expect(Docker).to receive(:version).and_raise(Excon::Errors::SocketError.new(StandardError.new)) + allow(Docker).to receive(:version).and_raise(Excon::Errors::SocketError.new(StandardError.new)) expect { described_class.check_availability! }.to raise_error(DockerClient::Error) end end context 'when a timeout occurs' do it 'raises an error' do - expect(Docker).to receive(:version).and_raise(Timeout::Error) + allow(Docker).to receive(:version).and_raise(Timeout::Error) expect { described_class.check_availability! }.to raise_error(DockerClient::Error) end end @@ -115,7 +113,7 @@ describe DockerClient, docker: true do context 'when retries are left' do before do - expect(described_class).to receive(:mapped_directories).and_raise(error).and_call_original + allow(described_class).to receive(:mapped_directories).and_raise(error).and_call_original end it 'retries to create a container' do @@ -125,7 +123,7 @@ describe DockerClient, docker: true do context 'when no retries are left' do before do - expect(described_class).to receive(:mapped_directories).exactly(DockerClient::RETRY_COUNT + 1).times.and_raise(error) + allow(described_class).to receive(:mapped_directories).exactly(DockerClient::RETRY_COUNT + 1).times.and_raise(error) end it 'raises the error' do @@ -140,7 +138,7 @@ describe DockerClient, docker: true do let(:container) { double } before do - expect(container).to receive(:binds).at_least(:once).and_return(["#{workspace_path}:#{DockerClient::CONTAINER_WORKSPACE_PATH}"]) + allow(container).to receive(:binds).at_least(:once).and_return(["#{workspace_path}:#{DockerClient::CONTAINER_WORKSPACE_PATH}"]) end after { docker_client.send(:create_workspace_files, container, submission) } @@ -183,11 +181,11 @@ describe DockerClient, docker: true do after { described_class.destroy_container(container) } it 'kills running processes' do - expect(container).to receive(:kill).and_return(container) + allow(container).to receive(:kill).and_return(container) end it 'releases allocated ports' do - expect(container).to receive(:port_bindings).at_least(:once).and_return(foo: [{'HostPort' => '42'}]) + allow(container).to receive(:port_bindings).at_least(:once).and_return(foo: [{'HostPort' => '42'}]) expect(PortPool).to receive(:release) end @@ -211,7 +209,7 @@ describe DockerClient, docker: true do end it 'sends the command' do - expect(docker_client).to receive(:send_command).with(command, kind_of(Docker::Container)).and_return({}) + allow(docker_client).to receive(:send_command).with(command, kind_of(Docker::Container)).and_return({}) execute_arbitrary_command end @@ -222,7 +220,7 @@ describe DockerClient, docker: true do let(:result) { {status: 'ok', stdout: 42} } before do - expect(docker_client).to receive(:send_command).and_raise(error).and_return(result) + allow(docker_client).to receive(:send_command).and_raise(error).and_return(result) end it 'retries to execute the command' do @@ -232,13 +230,13 @@ describe DockerClient, docker: true do context 'when no retries are left' do before do - expect(docker_client).to receive(:send_command).exactly(DockerClient::RETRY_COUNT + 1).times.and_raise(error) + allow(docker_client).to receive(:send_command).exactly(DockerClient::RETRY_COUNT + 1).times.and_raise(error) end it 'raises the error' do pending('retries are disabled') - # !TODO Retries is disabled - # expect { execute_arbitrary_command }.to raise_error(error) + # TODO: Retries is disabled + expect { execute_arbitrary_command }.to raise_error(error) end end end @@ -250,12 +248,10 @@ describe DockerClient, docker: true do after { docker_client.send(:execute_run_command, submission, filename) } it 'takes a container from the pool' do - pending('todo in the future') expect(DockerContainerPool).to receive(:get_container).with(submission.execution_environment).and_call_original end it 'creates the workspace files' do - pending('todo in the future') expect(docker_client).to receive(:create_workspace_files) end @@ -281,7 +277,7 @@ describe DockerClient, docker: true do it 'executes the test command' do expect(submission.execution_environment).to receive(:test_command).and_call_original - expect(docker_client).to receive(:send_command).with(kind_of(String), kind_of(Docker::Container)).and_return({}) + allow(docker_client).to receive(:send_command).with(kind_of(String), kind_of(Docker::Container)).and_return({}) end end @@ -305,7 +301,7 @@ describe DockerClient, docker: true do end context 'with incomplete configuration' do - before { expect(described_class).to receive(:config).at_least(:once).and_return({}) } + before { allow(described_class).to receive(:config).at_least(:once).and_return({}) } it 'raises an error' do expect { described_class.initialize_environment }.to raise_error(DockerClient::Error) @@ -357,7 +353,7 @@ describe DockerClient, docker: true do end describe '#send_command' do - let(:block) { proc {} } + let(:block) { proc { nil } } let(:container) { described_class.create_container(execution_environment) } let(:send_command) { docker_client.send(:send_command, command, container, &block) } @@ -379,8 +375,13 @@ describe DockerClient, docker: true do context 'when a timeout occurs' do before do - expect(container).to receive(:exec).once.and_raise(Timeout::Error) - expect(container).to receive(:exec).twice.and_return([[], []]) + exec_called = 0 + allow(container).to receive(:exec) do + exec_called += 1 + raise Timeout::Error if exec_called == 1 + + [[], []] + end end it 'destroys the container asynchronously' do @@ -410,5 +411,3 @@ describe DockerClient, docker: true do end end end - -# rubocop:enable RSpec/MultipleMemoizedHelpers diff --git a/spec/lib/docker_container_mixin_spec.rb b/spec/lib/docker_container_mixin_spec.rb index 6a3f04f3..8faaf8da 100644 --- a/spec/lib/docker_container_mixin_spec.rb +++ b/spec/lib/docker_container_mixin_spec.rb @@ -11,7 +11,7 @@ describe DockerContainerMixin do end it 'returns the correct information' do - expect(CONTAINER).to receive(:json).and_return('HostConfig' => {'Binds' => binds}) + allow(CONTAINER).to receive(:json).and_return('HostConfig' => {'Binds' => binds}) expect(CONTAINER.binds).to eq(binds) end end @@ -25,7 +25,7 @@ describe DockerContainerMixin do end it 'returns the correct information' do - expect(CONTAINER).to receive(:json).and_return('HostConfig' => {'PortBindings' => port_bindings}) + allow(CONTAINER).to receive(:json).and_return('HostConfig' => {'PortBindings' => port_bindings}) expect(CONTAINER.port_bindings).to eq(port => port) end end diff --git a/spec/lib/file_tree_spec.rb b/spec/lib/file_tree_spec.rb index 53361d79..8ef6ad7e 100644 --- a/spec/lib/file_tree_spec.rb +++ b/spec/lib/file_tree_spec.rb @@ -8,8 +8,8 @@ describe FileTree do describe '#file_icon' do let(:file_icon) { file_tree.send(:file_icon, file) } - context 'for a media file' do - context 'for an audio file' do + context 'with a media file' do + context 'with an audio file' do let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_mp3)) } it 'is an audio file icon' do @@ -17,7 +17,7 @@ describe FileTree do end end - context 'for an image file' do + context 'with an image file' do let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_jpg)) } it 'is an image file icon' do @@ -25,7 +25,7 @@ describe FileTree do end end - context 'for a video file' do + context 'with a video file' do let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_mp4)) } it 'is a video file icon' do @@ -34,8 +34,8 @@ describe FileTree do end end - context 'for other files' do - context 'for a read-only file' do + context 'with other files' do + context 'with a read-only file' do let(:file) { FactoryBot.build(:file, read_only: true) } it 'is a lock icon' do @@ -43,7 +43,7 @@ describe FileTree do end end - context 'for an executable file' do + context 'with an executable file' do let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_py)) } it 'is a code file icon' do @@ -51,7 +51,7 @@ describe FileTree do end end - context 'for a renderable file' do + context 'with a renderable file' do let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_svg)) } it 'is a text file icon' do @@ -59,7 +59,7 @@ describe FileTree do end end - context 'for all other files' do + context 'with all other files' do let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_md)) } it 'is a generic file icon' do @@ -100,7 +100,7 @@ describe FileTree do let!(:leaf) { root.add(Tree::TreeNode.new('', file)) } let(:root) { Tree::TreeNode.new('', file) } - context 'for a leaf node' do + context 'with a leaf node' do let(:node) { leaf } it 'produces the required attributes' do @@ -116,7 +116,7 @@ describe FileTree do end end - context 'for a non-leaf node' do + context 'with a non-leaf node' do let(:node) { root } it "traverses the node's children" do @@ -144,7 +144,7 @@ describe FileTree do let(:node_icon) { file_tree.send(:node_icon, node) } let(:root) { Tree::TreeNode.new('') } - context 'for the root node' do + context 'with the root node' do let(:node) { root } it 'is a folder icon' do @@ -152,7 +152,7 @@ describe FileTree do end end - context 'for leaf nodes' do + context 'with leaf nodes' do let(:node) { root.add(Tree::TreeNode.new('')) } it 'is a file icon' do @@ -161,7 +161,7 @@ describe FileTree do end end - context 'for intermediary nodes' do + context 'with intermediary nodes' do let(:node) do root.add(Tree::TreeNode.new('').tap {|node| node.add(Tree::TreeNode.new('')) }) end diff --git a/spec/lib/generators/testing_framework_adapter_generator_spec.rb b/spec/lib/generators/testing_framework_adapter_generator_spec.rb index ed4ab02b..6cf247d4 100644 --- a/spec/lib/generators/testing_framework_adapter_generator_spec.rb +++ b/spec/lib/generators/testing_framework_adapter_generator_spec.rb @@ -27,7 +27,7 @@ describe TestingFrameworkAdapterGenerator do it 'builds a correct class skeleton' do file_content = File.new(path, 'r').read - expect(file_content).to start_with("class #{name}Adapter < TestingFrameworkAdapter") + expect(file_content&.strip).to start_with("class #{name}Adapter < TestingFrameworkAdapter") end it 'generates a corresponding test' do diff --git a/spec/lib/nonce_store_spec.rb b/spec/lib/nonce_store_spec.rb index 145037dd..34252813 100644 --- a/spec/lib/nonce_store_spec.rb +++ b/spec/lib/nonce_store_spec.rb @@ -5,6 +5,10 @@ require 'rails_helper' describe NonceStore do let(:nonce) { SecureRandom.hex } + before do + stub_const('Lti::MAXIMUM_SESSION_AGE', 1) + end + describe '.add' do it 'stores a nonce in the cache' do expect(Rails.cache).to receive(:write) @@ -28,8 +32,6 @@ describe NonceStore do end it 'returns false for expired nonces' do - Lti.send(:remove_const, 'MAXIMUM_SESSION_AGE') - Lti::MAXIMUM_SESSION_AGE = 1 described_class.add(nonce) expect(described_class.has?(nonce)).to be true sleep(Lti::MAXIMUM_SESSION_AGE) diff --git a/spec/lib/testing_framework_adapter_spec.rb b/spec/lib/testing_framework_adapter_spec.rb index 77a7210d..cefb9868 100644 --- a/spec/lib/testing_framework_adapter_spec.rb +++ b/spec/lib/testing_framework_adapter_spec.rb @@ -42,7 +42,7 @@ describe TestingFrameworkAdapter do describe '#test_outcome' do it 'calls the framework-specific implementation' do - expect(adapter).to receive(:parse_output).and_return(count: count, failed: failed, passed: passed) + allow(adapter).to receive(:parse_output).and_return(count: count, failed: failed, passed: passed) adapter.test_outcome('') end end diff --git a/spec/models/code_ocean/file_spec.rb b/spec/models/code_ocean/file_spec.rb index 514ce987..baef520d 100644 --- a/spec/models/code_ocean/file_spec.rb +++ b/spec/models/code_ocean/file_spec.rb @@ -25,7 +25,7 @@ describe CodeOcean::File do expect(file.errors[:read_only]).to be_blank end - context 'as a teacher-defined test' do + context 'with a teacher-defined test' do before { file.update(role: 'teacher_defined_test') } it 'validates the presence of a feedback message' do diff --git a/spec/models/execution_environment_spec.rb b/spec/models/execution_environment_spec.rb index 396cbd96..b2edd346 100644 --- a/spec/models/execution_environment_spec.rb +++ b/spec/models/execution_environment_spec.rb @@ -6,7 +6,7 @@ describe ExecutionEnvironment do let(:execution_environment) { described_class.create.tap {|execution_environment| execution_environment.update(network_enabled: nil) } } it 'validates that the Docker image works', docker: true do - expect(execution_environment).to receive(:validate_docker_image?).and_return(true) + allow(execution_environment).to receive(:validate_docker_image?).and_return(true) expect(execution_environment).to receive(:working_docker_image?) execution_environment.update(docker_image: FactoryBot.attributes_for(:ruby)[:docker_image]) end @@ -124,22 +124,22 @@ describe ExecutionEnvironment do describe '#working_docker_image?', docker: true do let(:working_docker_image?) { execution_environment.send(:working_docker_image?) } - before { expect(DockerClient).to receive(:find_image_by_tag).and_return(Object.new) } + before { allow(DockerClient).to receive(:find_image_by_tag).and_return(Object.new) } it 'instantiates a Docker client' do expect(DockerClient).to receive(:new).with(execution_environment: execution_environment).and_call_original - expect_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_return({}) + allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_return({}) working_docker_image? end it 'executes the validation command' do - expect_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).with(ExecutionEnvironment::VALIDATION_COMMAND).and_return({}) + allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).with(ExecutionEnvironment::VALIDATION_COMMAND).and_return({}) working_docker_image? end context 'when the command produces an error' do it 'adds an error' do - expect_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_return(stderr: 'command not found') + allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_return(stderr: 'command not found') working_docker_image? expect(execution_environment.errors[:docker_image]).to be_present end @@ -147,7 +147,7 @@ describe ExecutionEnvironment do context 'when the Docker client produces an error' do it 'adds an error' do - expect_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_raise(DockerClient::Error) + allow_any_instance_of(DockerClient).to receive(:execute_arbitrary_command).and_raise(DockerClient::Error) working_docker_image? expect(execution_environment.errors[:docker_image]).to be_present end diff --git a/spec/models/exercise_spec.rb b/spec/models/exercise_spec.rb index c10b88d9..86596907 100644 --- a/spec/models/exercise_spec.rb +++ b/spec/models/exercise_spec.rb @@ -118,9 +118,7 @@ describe Exercise do end it 'duplicates all associated files' do - exercise.files.each do |file| - expect(file).to receive(:dup).and_call_original - end + expect(exercise.files).to all(receive(:dup).and_call_original) end it 'returns the duplicated exercise' do diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index 89f6d5ce..3e67d5d0 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -34,7 +34,7 @@ describe Submission do before { submission.score = submission.exercise.maximum_score / 2 } it 'returns the score as a value between 0 and 1' do - expect(0..1).to include(submission.normalized_score) + expect(submission.normalized_score).to be_between(0, 1) end end @@ -54,7 +54,7 @@ describe Submission do before { submission.score = submission.exercise.maximum_score / 2 } it 'returns the score expressed as a percentage' do - expect(0..100).to include(submission.percentage) + expect(submission.percentage).to be_between(0, 100) end end diff --git a/spec/policies/admin/dashboard_policy_spec.rb b/spec/policies/admin/dashboard_policy_spec.rb index a4a7a4de..4646d90b 100644 --- a/spec/policies/admin/dashboard_policy_spec.rb +++ b/spec/policies/admin/dashboard_policy_spec.rb @@ -3,19 +3,19 @@ require 'rails_helper' describe Admin::DashboardPolicy do - subject { described_class } + subject(:policy) { described_class } permissions :show? do it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), :dashboard) + expect(policy).to permit(FactoryBot.build(:admin), :dashboard) end it 'does not grant access to teachers' do - expect(subject).not_to permit(FactoryBot.build(:teacher), :dashboard) + expect(policy).not_to permit(FactoryBot.build(:teacher), :dashboard) end it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryBot.build(:external_user), :dashboard) + expect(policy).not_to permit(FactoryBot.build(:external_user), :dashboard) end end end diff --git a/spec/policies/code_ocean/file_policy_spec.rb b/spec/policies/code_ocean/file_policy_spec.rb index 8f65e928..f32b9b89 100644 --- a/spec/policies/code_ocean/file_policy_spec.rb +++ b/spec/policies/code_ocean/file_policy_spec.rb @@ -3,86 +3,86 @@ require 'rails_helper' describe CodeOcean::FilePolicy do - subject { described_class } + subject(:policy) { described_class } let(:exercise) { FactoryBot.create(:fibonacci) } let(:submission) { FactoryBot.create(:submission) } permissions :create? do - context 'as part of an exercise' do + context 'when being part of an exercise' do let(:file) { exercise.files.first } it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), file) + expect(policy).to permit(FactoryBot.build(:admin), file) end it 'grants access to authors' do - expect(subject).to permit(exercise.author, file) + expect(policy).to permit(exercise.author, file) end it 'does not grant access to all other users' do %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), file) + expect(policy).not_to permit(FactoryBot.build(factory_name), file) end end end - context 'as part of a submission' do + context 'when being part of a submission' do let(:file) { submission.files.first } - context 'where file creation is allowed' do + context 'when file creation is allowed' do before do submission.exercise.update(allow_file_creation: true) end it 'grants access to authors' do - expect(subject).to permit(submission.author, file) + expect(policy).to permit(submission.author, file) end end - context 'where file creation is not allowed' do + context 'when file creation is not allowed' do before do submission.exercise.update(allow_file_creation: false) end it 'grants access to authors' do - expect(subject).not_to permit(submission.author, file) + expect(policy).not_to permit(submission.author, file) end end it 'does not grant access to all other users' do %i[admin external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), file) + expect(policy).not_to permit(FactoryBot.build(factory_name), file) end end end end permissions :destroy? do - context 'as part of an exercise' do + context 'when being part of an exercise' do let(:file) { exercise.files.first } it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), file) + expect(policy).to permit(FactoryBot.build(:admin), file) end it 'grants access to authors' do - expect(subject).to permit(exercise.author, file) + expect(policy).to permit(exercise.author, file) end it 'does not grant access to all other users' do %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), file) + expect(policy).not_to permit(FactoryBot.build(factory_name), file) end end end - context 'as part of a submission' do + context 'when being part of a submission' do let(:file) { submission.files.first } it 'does not grant access to anyone' do %i[admin external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), file) + expect(policy).not_to permit(FactoryBot.build(factory_name), file) end end end diff --git a/spec/policies/consumer_policy_spec.rb b/spec/policies/consumer_policy_spec.rb index 832f6201..bd9071a6 100644 --- a/spec/policies/consumer_policy_spec.rb +++ b/spec/policies/consumer_policy_spec.rb @@ -3,14 +3,14 @@ require 'rails_helper' describe ConsumerPolicy do - subject { described_class } + subject(:policy) { described_class } %i[create? destroy? edit? index? new? show? update?].each do |action| permissions(action) do it 'grants access to admins only' do - expect(subject).to permit(FactoryBot.build(:admin), Consumer.new) + expect(policy).to permit(FactoryBot.build(:admin), Consumer.new) %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), Consumer.new) + expect(policy).not_to permit(FactoryBot.build(factory_name), Consumer.new) end end end diff --git a/spec/policies/execution_environment_policy_spec.rb b/spec/policies/execution_environment_policy_spec.rb index d640f2e8..41715025 100644 --- a/spec/policies/execution_environment_policy_spec.rb +++ b/spec/policies/execution_environment_policy_spec.rb @@ -3,22 +3,22 @@ require 'rails_helper' describe ExecutionEnvironmentPolicy do - subject { described_class } + subject(:policy) { described_class } let(:execution_environment) { FactoryBot.build(:ruby) } [:index?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), execution_environment) + expect(policy).to permit(FactoryBot.build(:admin), execution_environment) end it 'grants access to teachers' do - expect(subject).to permit(FactoryBot.build(:teacher), execution_environment) + expect(policy).to permit(FactoryBot.build(:teacher), execution_environment) end it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryBot.build(:external_user), execution_environment) + expect(policy).not_to permit(FactoryBot.build(:external_user), execution_environment) end end end @@ -26,16 +26,16 @@ describe ExecutionEnvironmentPolicy do %i[execute_command? shell? statistics? show?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), execution_environment) + expect(policy).to permit(FactoryBot.build(:admin), execution_environment) end it 'grants access to authors' do - expect(subject).to permit(execution_environment.author, execution_environment) + expect(policy).to permit(execution_environment.author, execution_environment) end it 'does not grant access to all other users' do %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), execution_environment) + expect(policy).not_to permit(FactoryBot.build(factory_name), execution_environment) end end end @@ -44,16 +44,16 @@ describe ExecutionEnvironmentPolicy do %i[destroy? edit? update? new? create?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), execution_environment) + expect(policy).to permit(FactoryBot.build(:admin), execution_environment) end it 'does not grant access to authors' do - expect(subject).not_to permit(execution_environment.author, execution_environment) + expect(policy).not_to permit(execution_environment.author, execution_environment) end it 'does not grant access to all other users' do %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), execution_environment) + expect(policy).not_to permit(FactoryBot.build(factory_name), execution_environment) end end end diff --git a/spec/policies/exercise_policy_spec.rb b/spec/policies/exercise_policy_spec.rb index b01d3688..3047d573 100644 --- a/spec/policies/exercise_policy_spec.rb +++ b/spec/policies/exercise_policy_spec.rb @@ -3,31 +3,31 @@ require 'rails_helper' describe ExercisePolicy do - subject { described_class } + subject(:policy) { described_class } let(:exercise) { FactoryBot.build(:dummy, public: true) } permissions :batch_update? do it 'grants access to admins only' do - expect(subject).to permit(FactoryBot.build(:admin), exercise) + expect(policy).to permit(FactoryBot.build(:admin), exercise) %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), exercise) + expect(policy).not_to permit(FactoryBot.build(factory_name), exercise) end end end - %i[create? index? new? statistics? feedback? get_rfcs_for_exercise?].each do |action| + %i[create? index? new? statistics? feedback? rfcs_for_exercise?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), exercise) + expect(policy).to permit(FactoryBot.build(:admin), exercise) end it 'grants access to teachers' do - expect(subject).to permit(FactoryBot.build(:teacher), exercise) + expect(policy).to permit(FactoryBot.build(:teacher), exercise) end it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryBot.build(:external_user), exercise) + expect(policy).not_to permit(FactoryBot.build(:external_user), exercise) end end end @@ -35,16 +35,16 @@ describe ExercisePolicy do %i[clone? destroy? edit? update?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), exercise) + expect(policy).to permit(FactoryBot.build(:admin), exercise) end it 'grants access to authors' do - expect(subject).to permit(exercise.author, exercise) + expect(policy).to permit(exercise.author, exercise) end it 'does not grant access to all other users' do %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), exercise) + expect(policy).not_to permit(FactoryBot.build(factory_name), exercise) end end end @@ -56,14 +56,14 @@ describe ExercisePolicy do let(:user) { exercise.author } it 'does not grant access' do - expect(subject).not_to permit(user, exercise) + expect(policy).not_to permit(user, exercise) end context 'when user has codeharbor_link' do before { user.codeharbor_link = FactoryBot.build(:codeharbor_link) } it 'grants access' do - expect(subject).to permit(user, exercise) + expect(policy).to permit(user, exercise) end end end @@ -72,14 +72,14 @@ describe ExercisePolicy do let(:user) { FactoryBot.build(:admin) } it 'does not grant access' do - expect(subject).not_to permit(user, exercise) + expect(policy).not_to permit(user, exercise) end context 'when user has codeharbor_link' do before { user.codeharbor_link = FactoryBot.build(:codeharbor_link) } it 'grants access' do - expect(subject).to permit(user, exercise) + expect(policy).to permit(user, exercise) end end end @@ -89,14 +89,14 @@ describe ExercisePolicy do let(:user) { FactoryBot.build(factory_name) } it 'does not grant access' do - expect(subject).not_to permit(user, exercise) + expect(policy).not_to permit(user, exercise) end context 'when user has codeharbor_link' do before { user.codeharbor_link = FactoryBot.build(:codeharbor_link) } it 'does not grant access' do - expect(subject).not_to permit(user, exercise) + expect(policy).not_to permit(user, exercise) end end end @@ -107,7 +107,7 @@ describe ExercisePolicy do [:show?].each do |action| permissions(action) do it 'not grants access to external users' do - expect(subject).not_to permit(FactoryBot.build(:external_user), exercise) + expect(policy).not_to permit(FactoryBot.build(:external_user), exercise) end end end @@ -116,7 +116,7 @@ describe ExercisePolicy do permissions(action) do it 'grants access to anyone' do %i[admin external_user teacher].each do |factory_name| - expect(subject).to permit(FactoryBot.build(factory_name), Exercise.new) + expect(policy).to permit(FactoryBot.build(factory_name), Exercise.new) end end end @@ -124,47 +124,47 @@ describe ExercisePolicy do describe ExercisePolicy::Scope do describe '#resolve' do - before(:all) do - @admin = FactoryBot.create(:admin) - @external_user = FactoryBot.create(:external_user) - @teacher = FactoryBot.create(:teacher) + let(:admin) { FactoryBot.create(:admin) } + let(:external_user) { FactoryBot.create(:external_user) } + let(:teacher) { FactoryBot.create(:teacher) } - [@admin, @teacher].each do |user| + before do + [admin, teacher].each do |user| [true, false].each do |public| FactoryBot.create(:dummy, public: public, user_id: user.id, user_type: InternalUser.name) end end end - context 'for admins' do - let(:scope) { Pundit.policy_scope!(@admin, Exercise) } + context 'when being an admin' do + let(:scope) { Pundit.policy_scope!(admin, Exercise) } it 'returns all exercises' do expect(scope.map(&:id)).to include(*Exercise.all.map(&:id)) end end - context 'for external users' do - let(:scope) { Pundit.policy_scope!(@external_user, Exercise) } + context 'when being an external users' do + let(:scope) { Pundit.policy_scope!(external_user, Exercise) } it 'returns nothing' do expect(scope.count).to be 0 end end - context 'for teachers' do - let(:scope) { Pundit.policy_scope!(@teacher, Exercise) } + context 'when being a teacher' do + let(:scope) { Pundit.policy_scope!(teacher, Exercise) } it 'includes all public exercises' do expect(scope.map(&:id)).to include(*Exercise.where(public: true).map(&:id)) end it 'includes all authored non-public exercises' do - expect(scope.map(&:id)).to include(*Exercise.where(public: false, user_id: @teacher.id).map(&:id)) + expect(scope.map(&:id)).to include(*Exercise.where(public: false, user_id: teacher.id).map(&:id)) end it "does not include other authors' non-public exercises" do - expect(scope.map(&:id)).not_to include(*Exercise.where(public: false).where("user_id <> #{@teacher.id}").map(&:id)) + expect(scope.map(&:id)).not_to include(*Exercise.where(public: false).where("user_id <> #{teacher.id}").map(&:id)) end end end diff --git a/spec/policies/external_user_policy_spec.rb b/spec/policies/external_user_policy_spec.rb index d907e209..82c1452b 100644 --- a/spec/policies/external_user_policy_spec.rb +++ b/spec/policies/external_user_policy_spec.rb @@ -3,14 +3,14 @@ require 'rails_helper' describe ExternalUserPolicy do - subject { described_class } + subject(:policy) { described_class } %i[create? destroy? edit? new? show? update?].each do |action| permissions(action) do it 'grants access to admins only' do - expect(subject).to permit(FactoryBot.build(:admin), ExternalUser.new) + expect(policy).to permit(FactoryBot.build(:admin), ExternalUser.new) %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), ExternalUser.new) + expect(policy).not_to permit(FactoryBot.build(factory_name), ExternalUser.new) end end end @@ -19,10 +19,10 @@ describe ExternalUserPolicy do [:index?].each do |action| permissions(action) do it 'grants access to admins and teachers only' do - expect(subject).to permit(FactoryBot.build(:admin), ExternalUser.new) - expect(subject).to permit(FactoryBot.build(:teacher), ExternalUser.new) + expect(policy).to permit(FactoryBot.build(:admin), ExternalUser.new) + expect(policy).to permit(FactoryBot.build(:teacher), ExternalUser.new) [:external_user].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), ExternalUser.new) + expect(policy).not_to permit(FactoryBot.build(factory_name), ExternalUser.new) end end end diff --git a/spec/policies/file_type_policy_spec.rb b/spec/policies/file_type_policy_spec.rb index f3e6edff..2b75f82b 100644 --- a/spec/policies/file_type_policy_spec.rb +++ b/spec/policies/file_type_policy_spec.rb @@ -3,23 +3,23 @@ require 'rails_helper' describe FileTypePolicy do - subject { described_class } + subject(:policy) { described_class } let(:file_type) { FactoryBot.build(:dot_rb) } %i[destroy? edit? update? new? create? index? show?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), file_type) + expect(policy).to permit(FactoryBot.build(:admin), file_type) end it 'grants access to authors' do - expect(subject).to permit(file_type.author, file_type) + expect(policy).to permit(file_type.author, file_type) end it 'does not grant access to all other users' do %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), file_type) + expect(policy).not_to permit(FactoryBot.build(factory_name), file_type) end end end diff --git a/spec/policies/internal_user_policy_spec.rb b/spec/policies/internal_user_policy_spec.rb index f4b4775a..fbbf3a0b 100644 --- a/spec/policies/internal_user_policy_spec.rb +++ b/spec/policies/internal_user_policy_spec.rb @@ -3,14 +3,14 @@ require 'rails_helper' describe InternalUserPolicy do - subject { described_class } + subject(:policy) { described_class } %i[create? edit? index? new? show? update?].each do |action| permissions(action) do it 'grants access to admins only' do - expect(subject).to permit(FactoryBot.build(:admin), InternalUser.new) + expect(policy).to permit(FactoryBot.build(:admin), InternalUser.new) %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), InternalUser.new) + expect(policy).not_to permit(FactoryBot.build(factory_name), InternalUser.new) end end end @@ -20,16 +20,16 @@ describe InternalUserPolicy do context 'with an admin user' do it 'grants access to no one' do %i[admin external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), FactoryBot.build(:admin)) + expect(policy).not_to permit(FactoryBot.build(factory_name), FactoryBot.build(:admin)) end end end context 'with a non-admin user' do it 'grants access to admins only' do - expect(subject).to permit(FactoryBot.build(:admin), InternalUser.new) + expect(policy).to permit(FactoryBot.build(:admin), InternalUser.new) %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), FactoryBot.build(:teacher)) + expect(policy).not_to permit(FactoryBot.build(factory_name), FactoryBot.build(:teacher)) end end end diff --git a/spec/policies/submission_policy_spec.rb b/spec/policies/submission_policy_spec.rb index fb1629f8..7d2aa4d6 100644 --- a/spec/policies/submission_policy_spec.rb +++ b/spec/policies/submission_policy_spec.rb @@ -3,12 +3,12 @@ require 'rails_helper' describe SubmissionPolicy do - subject { described_class } + subject(:policy) { described_class } permissions :create? do it 'grants access to anyone' do %i[admin external_user teacher].each do |factory_name| - expect(subject).to permit(FactoryBot.build(factory_name), Submission.new) + expect(policy).to permit(FactoryBot.build(factory_name), Submission.new) end end end @@ -16,21 +16,21 @@ describe SubmissionPolicy do %i[download_file? render_file? run? score? show? statistics? stop? test?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryBot.build(:admin), Submission.new) + expect(policy).to permit(FactoryBot.build(:admin), Submission.new) end it 'grants access to authors' do user = FactoryBot.create(:external_user) - expect(subject).to permit(user, FactoryBot.build(:submission, exercise: Exercise.new, user_id: user.id, user_type: user.class.name)) + expect(policy).to permit(user, FactoryBot.build(:submission, exercise: Exercise.new, user_id: user.id, user_type: user.class.name)) end end end permissions :index? do it 'grants access to admins only' do - expect(subject).to permit(FactoryBot.build(:admin), Submission.new) + expect(policy).to permit(FactoryBot.build(:admin), Submission.new) %i[external_user teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), Submission.new) + expect(policy).not_to permit(FactoryBot.build(factory_name), Submission.new) end end end diff --git a/spec/services/proforma_service/convert_task_to_exercise_spec.rb b/spec/services/proforma_service/convert_task_to_exercise_spec.rb index 733775ff..6e28b341 100644 --- a/spec/services/proforma_service/convert_task_to_exercise_spec.rb +++ b/spec/services/proforma_service/convert_task_to_exercise_spec.rb @@ -48,7 +48,7 @@ describe ProformaService::ConvertTaskToExercise do let(:files) { [] } let(:tests) { [] } let(:model_solutions) { [] } - let(:exercise) {} + let(:exercise) { nil } it 'creates an exercise with the correct attributes' do expect(convert_to_exercise_service).to have_attributes( @@ -83,7 +83,7 @@ describe ProformaService::ConvertTaskToExercise do let(:mimetype) { 'mimetype' } let(:binary) { false } let(:content) { 'content' } - let(:path) {} + let(:path) { nil } it 'creates an exercise with a file that has the correct attributes' do expect(convert_to_exercise_service.files.first).to have_attributes( @@ -196,11 +196,11 @@ describe ProformaService::ConvertTaskToExercise do let(:model_solution2) do Proforma::ModelSolution.new( id: 'ms-id-2', - files: ms_files_2 + files: ms_files2 ) end - let(:ms_files_2) { [ms_file_2] } - let(:ms_file_2) do + let(:ms_files2) { [ms_file2] } + let(:ms_file2) do Proforma::TaskFile.new( id: 'ms-file-2', content: 'content', diff --git a/spec/services/proforma_service/import_spec.rb b/spec/services/proforma_service/import_spec.rb index 98b074cc..8f85cf75 100644 --- a/spec/services/proforma_service/import_spec.rb +++ b/spec/services/proforma_service/import_spec.rb @@ -33,7 +33,7 @@ describe ProformaService::Import do user: user) end - let(:uuid) {} + let(:uuid) { nil } let(:execution_environment) { FactoryBot.build(:java) } let(:files) { [] } let(:tests) { [] } diff --git a/spec/support/docker.rb b/spec/support/docker.rb index 481fa343..4410edb1 100644 --- a/spec/support/docker.rb +++ b/spec/support/docker.rb @@ -21,7 +21,7 @@ RSpec.configure do |config| has_docker_tests = examples.any? {|example| example.metadata[:docker] } next unless has_docker_tests - FileUtils.rm_rf(Rails.root.join('tmp', 'files', 'test')) + FileUtils.rm_rf(Rails.root.join('tmp/files/test')) `which docker && test -n "$(docker ps --all --quiet)" && docker rm --force $(docker ps --all --quiet)` end end diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index 29dc2d08..4638bfff 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' describe FileUploader do - let(:file_path) { Rails.root.join('db', 'seeds', 'fibonacci', 'exercise.rb') } + let(:file_path) { Rails.root.join('db/seeds/fibonacci/exercise.rb') } let(:uploader) { described_class.new(FactoryBot.create(:file)) } before { uploader.store!(File.open(file_path, 'r')) }