CSC/ECE 517 Spring 2022 - E2200: Testing advice controller

About Expertiza

Expertiza is a software which benefits both instructors and students by providing platform for various types of submissions and providing reusable objects for peer review. Expertiza is an open-source project developed on Ruby on Rails framework. In Expertiza an instructors can not only create and customize new or existing assignments, but he/she can also create a list of topics and subject in which the students can sign up. Along with that, Students can form their teams and groups to work with on various projects and assignments. Students can also peer-review other students' submissions. This enables students to work together to improve each other’s learning experiences by providing feedbacks.

Description about project

This project is a description of Expertiza OSS project E2200 which is adding unit tests for advice_controller.rb and participants_helper.rb.

Advice in Expertiza is the comments corresponding to each score as given by the user. Suppose a user gives 4 score to a person. So, suppose there's an advice "Neat work but more explanation needed" corresponding to score 4. So, the user gave this comment along with score 4 to that person. Advice is the guidance corresponding to each and every score level for each rubric item.

The project focusses on refactoring and testing each aspect of advice_controller and participants_helper.rb by writing comprehensive test cases using rspec.

advice_controller first checks whether current user has TA privileges or not by implementing action_allowed? method. Secondly it sets the number of advices based on score and sort it in descending order. Then it checks four conditions for the advices.  

1.     If number of advices is not equal to given advices

2.     If the sorted advices is empty

3.     If first advice score of sorted advices is NOT equal to max score

4.     If last advice score of sorted advices is NOT equal to min score

If any of the above condition are True, the edit_advice method calls adjust_advice_size of the QuestionnaireHelper class which adjust the advice sizes accordingly.

In the end, save_advice method is called which updates and saves the changes in the advices and displays the success/failure message.

participants_helper.rb separates the file into the necessary elements to create a new user. It first defines the attributes and types of users and its parameters. Then participants_helper has methods to create new users and add users to assignment and course. Then, the helper file has methods to store items, get configurations and set permissions for participants.

In the end, the helper file sets permissions for various participants based on various aspects like can_submit, can_review or can_take_quiz.

Files Involved:

• advice_controller.rb
• advice_controller_spec.rb

• participants_helper.rb
• participants_helper_spec.rb

Running Tests:

• rspec spec/controllers/advice_controller_spec.rb
• rspec spec/helpers/participants_helper_spec.rb

Refactoring advice_controller

We understood the functionality of advice_controller and found a number of bugs in the code. So first we refactored the code by solving out the bugs and verifying it with the mentor. The bugs we found are enlisted below.

1. Change naming convention of num_questions to num_advices.

2. Adding 1 to the calculation of num_advices (max_score - min_score + 1)

3. Max should be compared with First element of sorted advice

4. Min should be compared with Last element of sorted advice

5. Score of Last element had to be compared instead of whole last element of sorted advice.

Test Plan for advice_controller

Initially we ran the original test cases but it only covered just a part of the methods in controllers. That is, it just covered the action_allowed part of out controller. Hence, we then wrote the tests for covering other methods of our controller. Our first method, invalid_advice? basically checks for 4 conditions for number of advices. So we wrote 5 tests considering result of each condition. Then we wrote 2 tests for save_advice method which checks whether advices are successfully saved or not.

We created the test cases using mocking of objects (factory and stub objects).

advice_controller Methods

The code of the controller can be found here. The methods are:

• action_allowed?
• invalid_advice?
• edit_advice
• save_advice

Test Frame for advice_controller

We write test cases in order to test the functional logic for the controllers.

1. action_allowed? – action_allowed? is the first method that is called when user tries to access the advice_controllers. It checks whether the current user has TA privileges or not.

Code Snippet:

Checks if current user is super-admin then allows certain action

describe '#action_allowed?' do
    context 'when the role of current user is Super-Admin' do
      it 'allows certain action' do
        stub_current_user(super_admin, super_admin.role.name, super_admin.role)
        expect(controller.send(:action_allowed?)).to be_truthy
      end
    end

Checks if current user is Instructor then allows certain action

   context 'when the role of current user is Instructor' do
      it 'allows certain action' do
        stub_current_user(instructor1, instructor1.role.name, instructor1.role)
        expect(controller.send(:action_allowed?)).to be_truthy
      end
    end

Checks if current user is Student then should not allow certain action

   context 'when the role of current user is Student' do
      it 'refuses certain action' do
        stub_current_user(student1, student1.role.name, student1.role)
        expect(controller.send(:action_allowed?)).to be_falsey
      end
    end
  end

2. invalid_advice? –

This method is called to check the 4 conditions in which the advice size has to be adjusted. If this function return true, only then we have to adjust the advice size.

Code Snippet:

Checks if invalid_advice? is correctly called with question advice score > max score of questionnaire

describe '#invalid_advice?' do
    context "when invalid_advice? is called with question advice score > max score of questionnaire" do
      #max score of advice = 3 (!=2)
      let(:qa1) {build(:question_advice, id:1, score: 1, question_id: 1, advice: "Advice1")}
      let(:qa2) {build(:question_advice, id:2, score: 3, question_id: 1, advice: "Advice2")}
      let(:questionnaire) do
        build(:questionnaire, id: 1, min_question_score: 1,
          questions: [build(:question, id: 1, weight: 2, question_advices: [qa1,qa2])], max_question_score: 2)
      end

      it "invalid_advice? returns true when called with incorrect maximum score for a question advice" do
        sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse
        num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1  
        temp = AdviceController.new
        temp.instance_variable_set(:@questionnaire,questionnaire)
        expect(temp.invalid_advice?(sorted_advice,num_advices,questionnaire.questions[0])).to eq(true)
      end
    end

Checks if invalid_advice? is correctly called with question advice score < min score of questionnaire

context "when invalid_advice? is called with question advice score < min score of questionnaire" do
      #min score of advice = 0 (!=1)
      let(:qa1) {build(:question_advice, id:1, score: 0, question_id: 1, advice: "Advice1")}
      let(:qa2) {build(:question_advice, id:2, score: 2, question_id: 1, advice: "Advice2")}
      let(:questionnaire) do
        build(:questionnaire, id: 1, min_question_score: 1,
          questions: [build(:question, id: 1, weight: 2, question_advices: [qa1,qa2])], max_question_score: 2)
      end

      it "invalid_advice? returns true when called with incorrect minimum score for a question advice" do
        sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse
        num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1  
        temp = AdviceController.new
        temp.instance_variable_set(:@questionnaire,questionnaire)
        expect(temp.invalid_advice?(sorted_advice,num_advices,questionnaire.questions[0])).to eq(true)
      end
    end

Checks if invalid_advice? is correctly called with number of advices > (max-min) score of questionnaire

context "when invalid_advice? is called with number of advices > (max-min) score of questionnaire" do
      #number of advices > 2
      let(:qa1) {build(:question_advice, id:1, score: 1, question_id: 1, advice: "Advice1")}
      let(:qa2) {build(:question_advice, id:2, score: 2, question_id: 1, advice: "Advice2")}
      let(:qa3) {build(:question_advice, id:3, score: 2, question_id: 1, advice: "Advice3")}
      let(:questionnaire) do
        build(:questionnaire, id: 1, min_question_score: 1,
          questions: [build(:question, id: 1, weight: 2, question_advices: [qa1,qa2,qa3])], max_question_score: 2)
      end

      it "invalid_advice? returns true when called with incorrect number of question advices" do
        sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse
        num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1  
        temp = AdviceController.new
        temp.instance_variable_set(:@questionnaire,questionnaire)
        expect(temp.invalid_advice?(sorted_advice,num_advices,questionnaire.questions[0])).to eq(true)
      end
    end

Checks if invalid_advice? is correctly called with no advices for a question in questionnaire

context "when invalid_advice? is called with no advices for a question in questionnaire" do
      # 0 advices - empty list scenario
      let(:questionnaire) do
        build(:questionnaire, id: 1, min_question_score: 1,
          questions: [build(:question, id: 1, weight: 2, question_advices: [])], max_question_score: 2)
      end

      it "invalid_advice? returns true when called with an empty advice list " do
        sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse
        num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1  
        temp = AdviceController.new
        temp.instance_variable_set(:@questionnaire,questionnaire)
        expect(temp.invalid_advice?(sorted_advice,num_advices,questionnaire.questions[0])).to eq(true)
      end
    end

Checks if invalid_advice? is correctly called with all conditions satisfied

context "when invalid_advice? is called with all conditions satisfied" do
      # all perfect
      let(:qa1) {build(:question_advice, id:1, score: 1, question_id: 1, advice: "Advice1")}
      let(:qa2) {build(:question_advice, id:2, score: 2, question_id: 1, advice: "Advice2")}
      let(:questionnaire) do
        build(:questionnaire, id: 1, min_question_score: 1,
          questions: [build(:question, id: 1, weight: 2, question_advices: [qa1,qa2])], max_question_score: 2)
      end

      it "invalid_advice? returns false when called with all correct pre-conditions " do
        sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse
        num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1  
        temp = AdviceController.new
        temp.instance_variable_set(:@questionnaire,questionnaire)
        expect(temp.invalid_advice?(sorted_advice,num_advices,questionnaire.questions[0])).to eq(false)
      end
    end
  end

3. edit_advice –

Test for this methods includes whether this method redirects correctly when advice size is to be adjusted. When invalid_advice? return true, edit_advice should call QuestionnaireHelper adjust_advice_size method and redirect to itself.

Code Snippet:

Checks when edit_advice is called and invalid_advice? evaluates to true then it redirects correctly

describe '#edit_advice' do

    context "when edit_advice is called and invalid_advice? evaluates to true" do
      # edit advice called
      let(:qa1) {build(:question_advice, id:1, score: 1, question_id: 1, advice: "Advice1")}
      let(:qa2) {build(:question_advice, id:2, score: 2, question_id: 1, advice: "Advice2")}
      let(:questionnaire) do
        build(:questionnaire, id: 1, min_question_score: 1,
          questions: [build(:question, id: 1, weight: 2, question_advices: [qa1,qa2])], max_question_score: 2)
      end

      it "edit advice redirects correctly when called" do
        allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire)
        params = {id: 1}
        session = {user: instructor1}
        result = get :edit_advice, params, session
        expect(result.status).to eq 200
        expect(result).to render_template(:edit_advice)
      end
    end
  end

4. save_advice –

This method saves the advices for a particular questionnaire. It updates the advice corresponding to particular key. The tests for this method includes whether it saves the advice successfully or not.

Code Snippet:

Checks when save_advice is called then advice saved successfully or not

describe '#save_advice' do
    context "when save_advice is called" do
      let(:qa1) {build(:question_advice, id:1, score: 1, question_id: 1, advice: "Advice1")}
      let(:qa2) {build(:question_advice, id:2, score: 2, question_id: 1, advice: "Advice2")}
      let(:questionnaire) do
        build(:questionnaire, id: 1, min_question_score: 1,
          questions: [build(:question, id: 1, weight: 2, question_advices: [qa1,qa2])], max_question_score: 2)
      end
      
      it "saves advice successfully" do
        allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire)
        allow(QuestionAdvice).to receive(:update).with('1',{:advice => "Hello"}).and_return("Ok")
        params = {advice: {"1" => {:advice => "Hello"}}, id: 1}
        session = {user: instructor1}
        result = get :save_advice, params, session
        expect(flash[:notice]).to eq('The advice was successfully saved!')
        expect(result.status).to eq 302
        expect(result).to redirect_to('/advice/edit_advice/1')
      end

      it "does not save the advice" do
        allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire)
        allow(QuestionAdvice).to receive(:update).with(any_args).and_return("Ok")
        params = {id: 1}
        session = {user: instructor1}
        result = get :save_advice, params, session
        expect(flash[:notice]).not_to be_present
        expect(result.status).to eq 302
        expect(result).to redirect_to('/advice/edit_advice/1')
      end
    end
  end
end

Refactoring participants_helper

We understood the functionality of participants_helper and found a bug in the code. So first we refactored the code by solving out the bug and verifying it with the mentor. The bug we found is enlisted below.

1. In define_attributes method, while assigning password attribute, the statement "attributes['password'] = assign_password(8)" doesn't work. there is no method named assign_password and hence manual password has to be initialized to generate an 8 character password.

Test Plan for participants_helper

There was no file named participants_helper_spec.rb. That is no test cases was pre written and hence we created a new file with the above name and added test cases from scratch for testing various aspects of participants_helper.rb file.

We have written the tests for covering maximum methods of our helper file.

We created the test cases using mocking of objects (factory and stub objects).

participants_helper Methods

The code of the helper can be found here. The methods are:

• upload_users
• define_attributes
• define_user
• create_new_user
• add_user_to_assignment
• add_user_to_course
• get_config
• store_item
• participant_permissions

Test Frame for participants_helper

We have written test cases in order to test the functional logic for the helper.

1. define_attributes – Our first method, define_attributes basically defines and assigns various attribute values for our participants. Hence we wrote the test case in order to check whether correct attribute values are defined/assigned or not.

Code Snippet:

Checks whether correct attribute values are defined/assigned or not.

describe '#define_attributes' do
        context 'when define_attributes is called' do
            #Checking if attributes have been correctly defined
            line_split = ['Test1','test@ncsu.edu']
            config = {'name'=>'0','fullname'=>'test2','email'=>'1'}

            it 'returns correct hash "attributes" when define_attributes is called' do
                allow(Role).to receive(:find_by).with({:name=>'Student'}).and_return(1)
                attribute = ParticipantsHelper.define_attributes(line_split,config)
                expect(attribute['role_id']).to eq(1)
                expect(attribute['name']).to eq('Test1')
                expect(attribute['fullname']).to eq('test2')
                expect(attribute['email']).to eq('test@ncsu.edu')
                expect(attribute['password'].length).to eq(8)
                expect(attribute['email_on_submission']).to eq(1)
                expect(attribute['email_on_review']).to eq(1)
                expect(attribute['email_on_review_of_review']).to eq(1)
            end
        end
    end

2. create_new_user –

Our Second method, create_new_user basically creates new user for a particular participant. So, we wrote a test case to check if correct user is created and called when create_new_user is called.

Code Snippet:

Checks if correct user is created and called when create_new_user is called.

describe '#create_new_user' do
        context 'when create_new_user is called' do
            #Checking if a user has een correctly created
            let(:instructor1) { build(:instructor, id: 10, role_id: 3, parent_id: 3, name: 'Instructor1') }
            
            it 'returns correct user when create_new_user is called' do
                attributes = {'role_id' => 1, 'name' => 'Test1', 'fullname' => 'test2', 'email' => 'test@ncsu.edu', 'email_on_submission' => 1, 'email_on_review' => 1, 'email_on_review_of_review' => 1}
                session = {user: instructor1}
                user = ParticipantsHelper.create_new_user(attributes, session)
                expect(user['role_id']).to eq(1)
                expect(user['name']).to eq('Test1')
                expect(user['fullname']).to eq('test2')
                expect(user['email']).to eq('test@ncsu.edu')
                expect(user['email_on_submission']).to eq(true)
                expect(user['email_on_review']).to eq(true)
                expect(user['email_on_review_of_review']).to eq(true)
                expect(user.parent_id).to eq(10)
            end
        end
    end

3. participant_permissions –

Our next method, participant_permissions basically sets permissions for various participants based on various aspects like can_submit, can_review or can_take_quiz according to authorization status of the participant. Hence we wrote test cases to check whether correct authorization and permission are given to each participant based on its authorization status.

Code Snippet:

Checks whether correct authorization and permission are given to each participant based on its authorization status.

describe '#participant_permissions' do
        before(:each) do
            include ParticipantsHelper
        end 

        context 'when participant_permissions is called' do
            it 'returns correct authorizations when participant_permissions is called with reader authorization' do
                #Checking permissions for a reader
                result = participant_permissions('reader')
                expect(result).to eq(can_submit: false, can_review: true, can_take_quiz: true)
            end

            it 'returns correct authorizations when participant_permissions is called with reviewer authorization' do
                #Checking permissions for a reviewer
                result = participant_permissions('reviewer')
                expect(result).to eq(can_submit: false, can_review: true, can_take_quiz: false)
            end

            it 'returns correct authorizations when participant_permissions is called with submitter authorization' do
                #Checking permissions for a submitter
                result = participant_permissions('submitter')
                expect(result).to eq(can_submit: true, can_review: false, can_take_quiz: false)
            end

            it 'returns correct authorizations when participant_permissions is called with paricipant authorization' do
                #Checking permissions for a participant
                result = participant_permissions('paricipant')
                expect(result).to eq(can_submit: true, can_review: true, can_take_quiz: true)
            end
        end
    end

4. store_item –

In the end we wrote a test case for store_item method. This test cases checks when store_item is called then whether config[ident] is properly assigned or not.

Code Snippet:

Checks when store_item is called then whether config[ident] is properly assigned or not.

describe '#define_attributes' do
describe '#store_item' do
        #checkingwhen store_item is called
        it 'assigns config[ident]' do
            #checking if config[ident] is properly assigned
            line = "test=Testing\nstore\nitem"
            ident = "test"
            config = {}
            ParticipantsHelper.store_item(line,ident,config)
            expect(config["test"]).to eq("Testingstore\nitem")
        end
    end

Results

1. Related Links:
   • A video of all tests running can be seen [here]
   • The main repository can be found [here].
   • The forked git repository for this project can be found [here].
2. Conclusion:

There were 4 modules in total in the advice_controller for which we wrote the unit tests following the behavior driven approach. We haven’t considered all the edge cases for now and hence there is always a scope to further improve the testing coverage to reach 100%.