Fix 17 previously failing specs

app/controllers/concerns/submission_scoring.rb

@@ -31,38 +31,27 @@ module SubmissionScoring
       LinterCheckRun.create_from(testrun, assessment)
       assessment = assessor.translate_linter(assessment, I18n.locale)
 
-          # replace file name with hint if linter is not used for grading. Refactor!
-          filename = t('exercises.implement.not_graded') if file.weight.zero?
-        end
+      # replace file name with hint if linter is not used for grading. Refactor!
+      filename = t('exercises.implement.not_graded', locale: :de) if file.weight.zero?
+    end
 
     output.merge!(assessment)
     output.merge!(filename: filename, message: feedback_message(file, output), weight: file.weight)
   end
 
-  private :collect_test_results
-
-  def execute_test_file(file, submission)
-    # TODO: replace DockerClient here
-    DockerClient.new(execution_environment: file.context.execution_environment).execute_test_command(submission,
-      file.name_with_extension)
-  end
-
-  private :execute_test_file
-
-  def feedback_message(_file, output)
-    # TODO: why did we comment out set_locale and render_markdown?
-    set_locale
+  # TODO: make this a controller concern again to bring the locales nearer to the view
+  def feedback_message(file, output)
     if output[:score] == Assessor::MAXIMUM_SCORE && output[:file_role] == 'teacher_defined_test'
       I18n.t('exercises.implement.default_test_feedback')
     elsif output[:score] == Assessor::MAXIMUM_SCORE && output[:file_role] == 'teacher_defined_linter'
       I18n.t('exercises.implement.default_linter_feedback')
     else
-      render_markdown(file.feedback_message)
+      # render_markdown(file.feedback_message)
+      file.feedback_message
     end
   end
 
   def score_submission(outputs)
-    # outputs = collect_test_results(submission)
     submission = self
     score = 0.0
     if outputs.present?

app/controllers/exercises_controller.rb

@@ -4,7 +4,6 @@ class ExercisesController < ApplicationController
   include CommonBehavior
   include Lti
   include SubmissionParameters
-  include SubmissionScoring
   include TimeHelper
 
   before_action :handle_file_uploads, only: %i[create update]
@@ -533,7 +532,7 @@ working_time_accumulated: working_time_accumulated})
 
   def submit
     @submission = Submission.create(submission_params)
-    score_submission(@submission)
+    @submission.calculate_score
     if @submission.user.external_user? && lti_outcome_service?(@submission.exercise_id, @submission.user.id)
       transmit_lti_score
     else

app/controllers/remote_evaluation_controller.rb

@@ -2,7 +2,6 @@
 
 class RemoteEvaluationController < ApplicationController
   include RemoteEvaluationParameters
-  include SubmissionScoring
   include Lti
 
   skip_after_action :verify_authorized
@@ -63,7 +62,7 @@ status: 202}
     validation_token = remote_evaluation_params[:validation_token]
     if (remote_evaluation_mapping = RemoteEvaluationMapping.find_by(validation_token: validation_token))
       @submission = Submission.create(build_submission_params(cause, remote_evaluation_mapping))
-      score_submission(@submission)
+      @submission.calculate_score
     else
       # TODO: better output
       # TODO: check token expired?

app/controllers/request_for_comments_controller.rb

@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 class RequestForCommentsController < ApplicationController
-  include SubmissionScoring
-
   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,
@@ -121,7 +119,7 @@ class RequestForCommentsController < ApplicationController
       if @request_for_comment.save
         # create thread here and execute tests. A run is triggered from the frontend and does not need to be handled here.
         Thread.new do
-          score_submission(@request_for_comment.submission)
+          @request_for_comment.submission.calculate_score
         ensure
           ActiveRecord::Base.connection_pool.release_connection
         end

app/controllers/submissions_controller.rb

@@ -5,7 +5,6 @@ class SubmissionsController < ApplicationController
   include CommonBehavior
   include Lti
   include SubmissionParameters
-  include SubmissionScoring
   include Tubesock::Hijack
 
   before_action :set_submission,
@@ -237,25 +236,21 @@ class SubmissionsController < ApplicationController
   end
 
   def score
-    Thread.new do
-      hijack do |tubesock|
-        return kill_socket(tubesock) if @embed_options[:disable_run]
+    hijack do |tubesock|
+      return kill_socket(tubesock) if @embed_options[:disable_run]
 
-        tubesock.send_data(@submission.calculate_score)
-        # To enable hints when scoring a submission, uncomment the next line:
-        # send_hints(tubesock, StructuredError.where(submission: @submission))
-      rescue Runner::Error::ExecutionTimeout => e
-        tubesock.send_data JSON.dump({cmd: :status, status: :timeout})
-        Rails.logger.debug { "Running a submission failed: #{e.message}" }
-      rescue Runner::Error => e
-        tubesock.send_data JSON.dump({cmd: :status, status: :container_depleted})
-        Rails.logger.debug { "Runner error while scoring a submission: #{e.message}" }
-      ensure
-        tubesock.send_data JSON.dump({cmd: :exit})
-        tubesock.close
-      end
+      tubesock.send_data(@submission.calculate_score)
+      # To enable hints when scoring a submission, uncomment the next line:
+      # send_hints(tubesock, StructuredError.where(submission: @submission))
+    rescue Runner::Error::ExecutionTimeout => e
+      tubesock.send_data JSON.dump({cmd: :status, status: :timeout})
+      Rails.logger.debug { "Running a submission failed: #{e.message}" }
+    rescue Runner::Error => e
+      tubesock.send_data JSON.dump({cmd: :status, status: :container_depleted})
+      Rails.logger.debug { "Runner error while scoring a submission: #{e.message}" }
     ensure
-      ActiveRecord::Base.connection_pool.release_connection
+      tubesock.send_data JSON.dump({cmd: :exit})
+      tubesock.close
     end
   end
 

spec/concerns/submission_scoring_spec.rb

@@ -2,36 +2,34 @@
 
 require 'rails_helper'
 
-class Controller < AnonymousController
-  include SubmissionScoring
-end
-
-# This is broken since the Runner was added.
-describe SubmissionScoring, skip: true do
-  let(:controller) { Controller.new }
+describe SubmissionScoring do
   let(:submission) { FactoryBot.create(:submission, cause: 'submit') }
 
-  before do
-    controller.instance_variable_set(:@current_user, FactoryBot.create(:external_user))
-    controller.instance_variable_set(:@_params, {})
-  end
-
   describe '#collect_test_results' do
-    after { controller.send(:collect_test_results, submission) }
+    let(:runner) { FactoryBot.create :runner }
+
+    before do
+      allow(Runner).to receive(:for).and_return(runner)
+      allow(runner).to receive(:copy_files)
+      allow(runner).to receive(:execute_interactively).and_return(1.0)
+    end
+
+    after { submission.calculate_score }
 
     it 'executes every teacher-defined test file' do
+      allow(submission).to receive(:score_submission)
       submission.collect_files.select(&:teacher_defined_assessment?).each do |file|
-        allow(controller).to receive(:execute_test_file).with(file, submission).and_return({})
+        allow(submission).to receive(:test_result).with(any_args, file).and_return({})
       end
     end
+
+    it 'scores the submission' do
+      allow(submission).to receive(:score_submission).and_return([])
+    end
   end
 
   describe '#score_submission', cleaning_strategy: :truncation do
-    after { controller.score_submission(submission) }
-
-    it 'collects the test results' do
-      allow(controller).to receive(:collect_test_results).and_return([])
-    end
+    after { submission.score_submission([]) }
 
     it 'assigns a score to the submissions' do
       expect(submission).to receive(:update).with(score: anything)

spec/controllers/exercises_controller_spec.rb

@@ -236,17 +236,35 @@ describe ExercisesController do
     expect_template(:statistics)
   end
 
-  # This is broken since the Runner was added.
-  describe 'POST #submit', skip: true do
+  describe 'POST #submit' 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(:scoring_response) do
+      [{
+        status: 'ok',
+        stdout: '',
+        stderr: '',
+        waiting_for_container_time: 0,
+        container_execution_time: 0,
+        file_role: 'teacher_defined_test',
+        count: 1,
+        failed: 0,
+        error_messages: [],
+        passed: 1,
+        score: 1.0,
+        filename: 'index.html_spec.rb',
+        message: 'Well done.',
+        weight: 2.0,
+      }]
+    end
 
     before do
       FactoryBot.create(:lti_parameter, external_user: user, exercise: exercise)
-      allow_any_instance_of(Submission).to receive(:normalized_score).and_return(1)
-      allow(controller).to receive(:collect_test_results).and_return([{score: 1, weight: 1}])
-      allow(controller).to receive(:score_submission).and_call_original
+      submission = FactoryBot.build(:submission, exercise: exercise, user: user)
+      allow(submission).to receive(:normalized_score).and_return(1)
+      allow(submission).to receive(:calculate_score).and_return(JSON.dump(scoring_response))
+      allow(Submission).to receive(:create).and_return(submission)
     end
 
     context 'when LTI outcomes are supported' do

spec/features/editor_spec.rb

@@ -94,10 +94,9 @@ describe 'Editor', js: true do
   end
 
   it 'contains a button for submitting the exercise' do
-    # This is broken since the Runner was added.
-    skip
-
-    allow_any_instance_of(SubmissionsController).to receive(:score_submission).and_return(scoring_response)
+    submission = FactoryBot.build(:submission, user: user, exercise: exercise)
+    allow(submission).to receive(:calculate_score).and_return(JSON.dump(scoring_response))
+    allow(Submission).to receive(:find).and_return(submission)
     click_button(I18n.t('exercises.editor.score'))
     expect(page).not_to have_css('#submit_outdated')
     expect(page).to have_css('#submit')