Apply manual rubocop fixes

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}

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

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)

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
 

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

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)

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

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)

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
 

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