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.

The project focusses on refactoring and testing each aspect of advice_controller 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.

Files Involved:

• advice_controller.rb
• advice_controller_spec.rb

Running Tests:

• rspec spec/controllers/advice_controller_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

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, is_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?
• is_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. is_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 is_invalid_advice? is correctly called with question advice score > max score of questionnaire

describe '#is_invalid_advice?' do
    context "when is_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 "is_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.is_invalid_advice?(sorted_advice,num_advices,questionnaire.questions[0])).to eq(true)
      end
    end

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

context "when is_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 "is_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.is_invalid_advice?(sorted_advice,num_advices,questionnaire.questions[0])).to eq(true)
      end
    end

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

context "when is_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 "is_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.is_invalid_advice?(sorted_advice,num_advices,questionnaire.questions[0])).to eq(true)
      end
    end

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

context "when is_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 "is_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.is_invalid_advice?(sorted_advice,num_advices,questionnaire.questions[0])).to eq(true)
      end
    end

Checks if is_invalid_advice? is correctly called with all conditions satisfied

context "when is_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 "is_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.is_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 is_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 is_invalid_advice? evaluates to true then it redirects correctly

describe '#edit_advice' do

    context "when edit_advice is called and is_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

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%.