CSC/ECE 517 Spring 2022 - E2207: Testing for submitted content controller.rb

About Expertiza

Expertiza is the software benefits for both instructors and students by supporting various types of submissions and providing reusable objects for peer review. It is an open-source project based on Ruby on Rails framework. It allows the instructors not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams to work on various projects and assignments. Expertiza also lets students peer-review other students' submissions, enabling them to work together to improve others' learning experiences.

Current Project Description

This Expertiza OSS project "E2207: Testing for submitted_content_controller" is focused on adding unit tests for submitted_content_controller.rb.

Files Involved

• submitted_content_controller.rb
• submitted_content_controller_spec.rb
• test files (helloworld.c and the-rspec-book_p2_1.pdf) located here

The test files are used to test functionality in the submit_file method.

Running Tests

Ensure the test files are located in /expertizabeta/spec/fixtures/files

 rspec ./spec/controllers/submitted_content_controller_spec.rb

Test Plan

The submitted_content_controller previously had 1 public method (submit_file) tested which was very minimal. Additionally, the controller has 8 public methods that had no previous testing, and 9 private methods.

This test plan only covers testing for the public methods. RSpec is a Behavior Driven Design (BDD) focused test language where code interface and behavior is tested, not implementation. Additionally in Sandi Metz's talk on The Magic Tricks of Testing, she warns against testing private methods as testing them binds you to the current implementation of the code. Most private methods do implicity get tested while testing the public methods.

The RSpec unit tests added for these methods in the spec file 'submitted_content_controller_spec.rb' are discussed below.

Submitted Content Controller Methods

The code of the controller testing can be found here

The public methods which are not tested are:

• remove_hyperlink
• folder_action

The private methods which are tested in a public method are:

• check_content_type_integrity - tested in submit_file public method
• check_file_size - tested in submit_file public method

The private methods which are not explicitly tested are:

• file_type
• move_selected_file
• rename_selected_file
• delete_selected_file
• copy_selected_file
• create_new_folder
• one_team_can_submit_work?

Test Code

action_allowed?

The action_allowed? method checks that the current user is a student with required authorizations for the edit, submit_file, and submit_hyperlink methods. The test approach was borrowed from E2205 and expanded on. It tests the return value of action_allowed? in 7 scenarios

Code snippet:

describe '#action_allowed?' do
  context 'current user is not authorized' do
    it 'does not allow action for no user' do
      expect(controller.send(:action_allowed?)).to be false
    end
    it 'does not allow action for student without authorizations' do
      allow(controller).to receive(:current_user).and_return(build(:student))
      expect(controller.send(:action_allowed?)).to be false
    end
  end
  context 'current user has needed privileges' do
    it 'allows edit action for student with needed authorizations' do
      stub_current_user(student1, student1.role.name, student1.role)
      allow(controller).to receive(:are_needed_authorizations_present?).and_return(true)
      controller.params = {action: 'edit'}
      expect(controller.send(:action_allowed?)).to be true
    end
    it 'allows submit file action for students with team that can submit' do
      stub_current_user(student1, student1.role.name, student1.role)
      allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
      controller.params = {action: 'submit_file'}
      expect(controller.send(:action_allowed?)).to be true
    end
    it 'allows submit hyperlink action for students with team that can submit' do
      stub_current_user(student1, student1.role.name, student1.role)
      allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
      controller.params = {action: 'submit_hyperlink'}
      expect(controller.send(:action_allowed?)).to be true
    end
    it 'allows action for admin' do
      stub_current_user(admin, admin.role.name, admin.role)
      expect(controller.send(:action_allowed?)).to be true
    end
    it 'allows action for super admin' do
      stub_current_user(super_admin, super_admin.role.name, super_admin.role)
      expect(controller.send(:action_allowed?)).to be true
    end
  end
end

controller_locale

We verify the controller_locale method returns the expected locale_for_student: I18n.default_locale

Code snippet:

describe '#controller_locale' do
  it 'should return I18n.default_locale' do
    user = student1
    stub_current_user(user, user.role.name, user.role)
    expect(controller.send(:controller_locale)).to eq(I18n.default_locale)
  end
end

edit

The function tests whether a user is allowed to perform edit actions, as dictated by the current_user_id matching the @participant.user_id and ensuring they have the proper edit rights. When successful, the call should render the ":edit" template.

Code snippet:

describe 'student#edit' do
  it 'student#edit it' do
    allow(AssignmentParticipant).to receive(:find).and_return(participant)
    allow(Participant).to receive(:find_by).and_return(participant)
    stub_current_user(student1, student1.role.name, student1.role)
    allow(participant).to receive(:name).and_return('Name')
    params = { id: 21 }
    response = get :edit, params: params
    expect(response).to render_template(:edit)
  end
end
describe 'instructor#edit' do
  it 'instructor#edit it' do
    allow(AssignmentParticipant).to receive(:find).and_return(participant)
    allow(Participant).to receive(:find_by).and_return(participant)
    stub_current_user(instructor1, instructor1.role.name, instructor1.role)
    allow(participant).to receive(:name).and_return('Name')
    params = { id: 21 }
    response = get :edit, params: params
    expect(response).to render_template(:edit)
  end
end
describe 'superadmin#edit' do
  it 'superadmin#edit it' do
    allow(AssignmentParticipant).to receive(:find).and_return(participant)
    allow(Participant).to receive(:find_by).and_return(participant)
    stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
    allow(participant).to receive(:name).and_return('Name')
    params = { id: 21 }
    response = get :edit, params: params
    expect(response).to render_template(:edit)
  end
end

view

The function first verifies if the user has rights to view the current contents by validating with current_user_id matching the @participant.user_id. If this is valid, then you are redirected to the edit action, except in the 'view: true' format.

Code snippet:

describe 'student#view' do
  it 'student#view it' do
    #puts('-----STUDENT#view--------')
    allow(AssignmentParticipant).to receive(:find).and_return(participant)
    stub_current_user(student1, student1.role.name, student1.role)
    allow(participant).to receive(:name).and_return('Name')
    params = { id: 21 }
    response = get :view, params: params
    expect(response).to redirect_to(action: :edit, view: true, id: 21)
  end
end
describe 'instructor#view' do
  it 'instructor#view it' do
    #puts('-----instructor#view--------')
    allow(AssignmentParticipant).to receive(:find).and_return(participant)
    stub_current_user(instructor1, instructor1.role.name, instructor1.role)
    allow(participant).to receive(:name).and_return('Name')
    params = { id: 21 }
    #puts('-------inst a-----------')
    response = get :view, params: params
    #puts('-------inst b-----------')
    expect(response).to redirect_to(action: :edit, view: true, id: 21)
  end
end
describe 'superadmin#view' do
  it 'superadmin#view it' do
    #puts('-----superadmin#view--------')
    allow(AssignmentParticipant).to receive(:find).and_return(participant)
    stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
    allow(participant).to receive(:name).and_return('Name')
    params = { id: 21 }
    response = get :view, params: params
    expect(response).to redirect_to(action: :edit, view: true, id: 21)
  end
end

submit hyperlink

We test the submit_hyperlink error checking by verifying that it flashes an error for duplicate hyperlinks and invalid URLs

Code snippet:

describe '#submit_hyperlink' do
  context 'current user is participant and submits hyperlink' do
    before(:each) do
      allow(AssignmentParticipant).to receive(:find).and_return(participant)
      stub_current_user(student1, student1.role.name, student1.role)
      allow(participant).to receive(:team).and_return(team)
      allow(participant).to receive(:name).and_return('Name')
    end
    it 'flashes error if a duplicate hyperlink is submitted' do
      allow(team).to receive(:hyperlinks).and_return(['google.com'])
      params = {submission: "google.com", id: 21}
      response = get :submit_hyperlink, params: params
      expect(response).to redirect_to(action: :edit, id: 1)
      expect(flash[:error]).to eq 'You or your teammate(s) have already submitted the same hyperlink.'
    end
    it 'flashes error if url is invalid' do
      allow(team).to receive(:hyperlinks).and_return([])
      params = {submission: "abc123", id: 21}
      response = get :submit_hyperlink, params: params
      expect(response).to redirect_to(action: :edit, id: 1)
      expect(flash[:error]).to be_present # not checking message content since it uses #{$ERROR_INFO}
    end
  end
end

remove hyperlink

We created 2 pending tests for this method, it doesn't appear to currently be used in the submitted_content_controller and may need work.

Code snippet:

describe '#remove_hyperlink' do
  #NOTE - this method is not currently used, the below context is a start
  #       at proposed tests that may be useful in the future
  context 'current user is participant' do
    before(:each) do
      #allow(AssignmentParticipant).to receive(:find).and_return(participant)
      #stub_current_user(student1, student1.role.name, student1.role)
      #allow(participant).to receive(:team).and_return(team)
      #allow(team).to receive(:hyperlinks).and_return(['google.com'])
    end
    it 'redirects to edit if submissions are allowed' #do
      #params = {id: 1}
      #allow(assignment).to receive(:submission_allowed).and_return(true)
      #response = get :remove_hyperlink, params
      #expect(response).to redirect_to(action: :edit, id: 1)
    #end
    it 'redirects to view if submissions are not allowed' #do
      #params = {id: 1}
      #allow(assignment).to receive(:submission_allowed).and_return(true)
      #response = get :remove_hyperlink, params
      #expect(response).to redirect_to(action: :view, id: 1)
    #end
  end
end

submit_file

Submit_file had one test prior to this project, however this test appears to have broken after the Expertiza Rails 5.1 migration. We added tests for error checking, verifying that an error is flashed if a file exceeding the size limit or a file with an unaccepted type is submitted. We did this using file fixtures and added two new files for test to \spec\fixtures\files. We chose to use file fixtures to keep these tests fast (F.I.R.S.T) as opposed to creating test files on the fly

Code snippet:

describe '#submit_file' do
  context 'current user does not match up with the participant' do
    # this test has problems after rails 5.1 migration, getting 'undefined method 'size' for nil:NilClass' error
    # when 'check_content_size' method is called in submitted_content_controller, doesn't seem to be a problem in
    # other submit_file tests that should call the same method though...
    it 'renders edit template' #do
      #allow(AssignmentParticipant).to receive(:find).and_return(participant)
      #stub_current_user(instructor1, instructor1.role.name, instructor1.role)
      #request_params = { id: 1 }
      #response = get :submit_file, params: request_params
      #expect(response).to redirect_to(action: :edit, id: 1)
    #end
  end
  context 'user that is participant uploads a file' do
    before(:each) do
      allow(AssignmentParticipant).to receive(:find).and_return(participant)
      stub_current_user(student1, student1.role.name, student1.role)
    end
    it 'flashes error for file exceeding size limit' do
      allow(controller).to receive(:check_content_size).and_return(false)
      file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/the-rspec-book_p2_1.pdf")
      params = {uploaded_file: file,
                id: 1}
      response = get :submit_file, params: params
      expect(response).to redirect_to(action: :edit, id: 1)
      expect(flash[:error]).to be_present # not checking message content since it uses variable size limit
    end
    it 'flashes error for file of unexpected type' do
      allow(SubmittedContentController).to receive(:check_content_type_integrity).and_return(false)
      allow(MimeMagic).to receive(:by_magic).and_return("not valid")
      allow_any_instance_of(Rack::Test::UploadedFile::String).to receive(:read).and_return("")
      file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/helloworld.c")
      params = {uploaded_file: file,
                  id: 1}
      response = get :submit_file, params: params
      expect(response).to redirect_to(action: :edit, id: 1)
      expect(flash[:error]).to eq 'File type error'
    end
    # we could test that file is written and submission record is created, but we could have to
    # make assumptions about how path is formed and user input for path/filename is sanitized.  We don't want
    # this test coupled to the existing implementation
  end
end

folder_action

We've added 6 pending tests for folder_action. The folder_action method actually manipulates files on the server so testing these methods may require some creativity. Using file fixtures (similar to testing done for submit_file method) may be a way to implement the pending tests.

Code snippet:

describe '#folder_action' do
  context 'current user does not match up with the participant' do
    #method just returns in this context, how do we test that?
  end
  context 'current user is participant performing folder action' do
    before(:each) do
      allow(AssignmentParticipant).to receive(:find).and_return(participant)
      stub_current_user(student1, student1.role.name, student1.role)
    end
    it 'redirects to edit' #do
      #params = {id: 1, faction: nil}
      #response = get :folder_action, params
      #expect(response).to redirect_to(action: :edit, id: 1)
    #end
    it 'delete action deletes selected files'
    it 'rename action renames selected file'
    it 'move action moves selected file'
    it 'copy action copies selected file'
    it 'create folder action creates new directory'
  end
end

download

We created 5 tests (one of them pending) to verify that the download method flashes errors for invalid path names and non-existant files

Code snippet:

describe '#download' do
  context 'user downloads file' do
    it 'flashes error for nil folder name' do
      params = {folder_name: nil}
      response = get :download, params: params
      expect(flash[:error]).to be_present # not checking message content since it uses exception message
    end
    it 'flashes error for nil file name' do
      params = {name: nil}
      response = get :download, params: params
      expect(flash[:error]).to be_present # not checking message content since it uses exception message
    end
    it 'flashes error if attempt is to download entire folder' do
      params = {folder_name: 'test_directory', name: nil}
      response = get :download, params: params
      expect(flash[:error]).to be_present # not checking message content since it uses exception message
    end
    it 'flashes error if file does not exist' do
      params = {folder_name: 'unlikely_dir_name', name: 'nonexistantfile.no'}
      response = get :download, params: params
      expect(flash[:error]).to be_present #not checking message content since it uses exception message
    end
    it 'calls send for a valid file download' do
      # still figuring this one out...
      #params = {folder_name: 'test_dir', name: 'test.txt'}
      #File.stub(:exist?).and_return(true)
      #response = get :download, params
      #expect(download).to receive(:send_file)
    end
  end
end

Conclusion

We finished with 27 implemented tests in 9 contexts, and 9 pending tests that could be implemented in the future.

Coverall coverage report (expertiza/coverage/index.html) reported 44.32% coverage. The coveralls bot on GitHub reported 51% on our 2nd pull request. We believe this number may be lower because private methods in the controller weren't explicitly tested.

The controller could use refactoring. Specifically, it appears the remove_hyperlink method is not currently used. It should be used if verified to work or removed.

Testing of the folder_action method should be done, but we were unable to get it working with the file fixtures approach.

Two pull requests were created for Expertiza (see related links). Our first pull request was accepted and merged into the beta branch. Our 2nd pull request is pending review. We are not aware of any automated build errors and the expertiza-travisci-bot commented that the changes look good on our latest pull request.

Related Links

The main repository can be found here.

The forked git repository for this project can be found here.

The pull request can be found here.

A 2nd pull request made, after the first pull request was accepted can be found here

A video of all tests running can be seen here.