CSC/ECE 517 Spring 2017/E1724: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
No edit summary
 
(8 intermediate revisions by 2 users not shown)
Line 7: Line 7:
'''Problem Statement'''
'''Problem Statement'''


Remove duplicated code in feature tests and improve the overall Code Climate.
Remove duplicated code in feature tests and improve the overall Code Climate. Code Climate aides in determining the DRYness and style of code, more information can be found at https://codeclimate.com/dashboard.
 
'''Running Tests'''
 
After building the Expertiza environment run 'rspec spec/features/*_spec.rb' to run all of the feature tests. This will run the feature spec test files.
 
'''Test Plan'''
 
Since this is a refactoring of rspec tests, the testing will consist of running the tests as described in the previous section and ensuring that all of the tests pass. In order to fully test that the refactoring has worked the negative of the expect statements where tested to ensure that they failed and that the tests were still running as expected. Once the test failed as expected the tests were changed back to run to success.


'''Refactoring Delayed_mailer Method'''  
'''Refactoring Delayed_mailer Method'''  
Line 14: Line 22:
This method took time stamps using time_parse function, which would create issues when users change their time zones. Calling time_zone_parse instead of time_parse solves the issue. This method also had massive duplicate code for different test scenarios. A helper method enqueue_delayed_job(stage) is used to encapsulate the task being performed.
This method took time stamps using time_parse function, which would create issues when users change their time zones. Calling time_zone_parse instead of time_parse solves the issue. This method also had massive duplicate code for different test scenarios. A helper method enqueue_delayed_job(stage) is used to encapsulate the task being performed.


After refactoring in delayed_mailer_spec.rb:
After refactoring in delayed_mailer_spec.rb each of the test cases were written in less than five lines of code.
 
    def enqueue_delayed_job(stage)
      #enqueue a delayed job using current stage’s timestamp
    end
 
    describe '<stage> deadline reminder email' do
      it 'is able to send reminder email for <stage> deadline to <stage_users> ' do
        enqueue_delayed_job(stage)
        expect(Delayed::Job.count).to eq(1)
        expect(Delayed::Job.last.handler).to include("deadline_type: <stage>")
      end
    end


'''Refactoring Scheduled_task_spec Method'''  
'''Refactoring Scheduled_task_spec Method'''  
Line 33: Line 29:
This method took time stamps using time_parse function, which would create issues when users change their time zones. Calling time_zone_parse instead of time_parse solves the issue. This method also had massive duplicate code for different test scenarios. A helper method enqueue_scheduled_tasks(stage) is used to encapsulate the task being performed.
This method took time stamps using time_parse function, which would create issues when users change their time zones. Calling time_zone_parse instead of time_parse solves the issue. This method also had massive duplicate code for different test scenarios. A helper method enqueue_scheduled_tasks(stage) is used to encapsulate the task being performed.


After refactoring in scheduled_spec.rb:
After refactoring in scheduled_spec.rb the majority of the test cases were written in less than five lines, with the exception of the team formation test which required additional logic but was still significantly compressed.
 
    def enqueue_scheduled_tasks(stage)
      #enqueue a delayed job using current stage’s timestamp
    end
 
    describe '<stage> deadline reminder email' do
      it 'is able to send reminder email for <stage> deadline to <stage_users> ' do
        enqueue_scheduled_tasks(stage)
        expect(Delayed::Job.count).to eq(1)
        expect(Delayed::Job.last.handler).to include("deadline_type: <stage>")
      end
    end


'''Refactoring Assignment_creation_spec'''  
'''Refactoring Assignment_creation_spec'''  


Assignment_creation_spec covers testing scenarios that create public and private assignments as well as the various options in this assignment creation.
Assignment_creation_spec covers testing scenarios that create public and private assignments as well as the various options in this assignment creation.
This using CodeClimate it was identified that a large portion of the code was duplicated across multiple test cases which violates the DRY principle. The redundant code was generalized and placed in methods instead of being written in each of the test cases and redundant methods that were never called were removed from class.
Using CodeClimate it was identified that a large portion of the code was duplicated across multiple test cases which violates the DRY principle. The redundant code was generalized and placed in methods instead of being written in each of the test cases and redundant methods that were never called were removed from class.


The code below is a sample of the refactored code where instead of having redundant code, handle_questionaire is called with a few parameters and all of the redundant code in the test cases is replaced.
The code below is a sample of the refactored code where instead of having redundant code, handle_questionaire is called with a few parameters and all of the redundant code in the test cases is replaced.


def validate_attributes(questionaire_name)
  def validate_attributes(questionaire_name)
  questionnaire = get_questionnaire(questionaire_name).first
    questionnaire = get_questionnaire(questionaire_name).first
  expect(questionnaire).to have_attributes(
    expect(questionnaire).to have_attributes(
    questionnaire_weight: 50,
      questionnaire_weight: 50,
    notification_limit: 50
      notification_limit: 50
  )
    )
end
  end


def validate_dropdown
  def validate_dropdown
  questionnaire = Questionnaire.where(name: "ReviewQuestionnaire2").first
    questionnaire = Questionnaire.where(name: "ReviewQuestionnaire2").first
  assignment_questionnaire = AssignmentQuestionnaire.where(assignment_id: @assignment.id, questionnaire_id: questionnaire.id).first
    assignment_questionnaire = AssignmentQuestionnaire.where(assignment_id: @assignment.id, questionnaire_id: questionnaire.id).first
  expect(assignment_questionnaire.dropdown).to eq(false)
    expect(assignment_questionnaire.dropdown).to eq(false)
end
  end


def fill_in_questionaire(questionaire_css, questionaire_name)
  def fill_in_questionaire(questionaire_css, questionaire_name)
  within(:css, questionaire_css) do
    within(:css, questionaire_css) do
    select questionaire_name, from: 'assignment_form[assignment_questionnaire][][questionnaire_id]'
      select questionaire_name, from: 'assignment_form[assignment_questionnaire][][questionnaire_id]'
    uncheck('dropdown')
      uncheck('dropdown')
    select "Scale", from: 'assignment_form[assignment_questionnaire][][dropdown]'
      select "Scale", from: 'assignment_form[assignment_questionnaire][][dropdown]'
    fill_in 'assignment_form[assignment_questionnaire][][questionnaire_weight]', with: '50'
      fill_in 'assignment_form[assignment_questionnaire][][questionnaire_weight]', with: '50'
    fill_in 'assignment_form[assignment_questionnaire][][notification_limit]', with: '50'
      fill_in 'assignment_form[assignment_questionnaire][][notification_limit]', with: '50'
    end
    click_button 'Save'
    sleep 1
   end
   end
  click_button 'Save'
  sleep 1
end


def handle_questionaire(questionaire_css, questionaire_name, test_attributes)
  def handle_questionaire(questionaire_css, questionaire_name, test_attributes)
  fill_in_questionaire(questionaire_css, questionaire_name)
    fill_in_questionaire(questionaire_css, questionaire_name)
  if test_attributes
    if test_attributes
    validate_attributes(questionaire_name)
      validate_attributes(questionaire_name)
  else
    else
     validate_dropdown
      validate_dropdown
     end
   end
   end
end




Line 93: Line 77:


Instructor_interface_spec covers testing scenarios like creating a course, importing tests, and viewing publishing rights.
Instructor_interface_spec covers testing scenarios like creating a course, importing tests, and viewing publishing rights.
Unlike assignment_creation_spec, the largest violation (as determined by CodeClimate) of the DRY principle was functionality that was duplicated in questionnaire_spec. In order to fix this /spec/helpers/instructor_interface_helper_spec was created as a module and then included in both of the other Ruby files as a mixin.
Unlike assignment_creation_spec, the largest violation (as determined by CodeClimate) of the DRY principle was functionality that was exactly duplicated in questionnaire_spec. In order to fix this /spec/helpers/instructor_interface_helper_spec was created as a module and then included in both of the other Ruby files as a mixin.
 
 
'''Refactoring questionnaire_spec'''
 
The questionnaire_spec covers testing scenarios relating to creating questionnaires and filling them out.  The unique thing about the questionnaire_spec was that it had a lot of instances of similar code, but not identical.  This is because there are many different types of questions and every variation has to be tested.  Aside from the specific question name, the process of testing the editing and deleting of each question type was the same.  In order to make this more generic and repeatable, a new method was created which took the question type as an input and an each operator was used to cycle through each type and to test editing and deletion.  Below is the definition which was created to test each question type for the ability to edit and delete.
 
  question_type = %w(Criterion Scale Dropdown Checkbox TextArea TextField UploadFile SectionHeader TableHeader ColumnHeader)
 
  def load_question question_type, verify_button
    load_questionnaire
    fill_in('question_total_num', with: '1')
    select(question_type, from: 'question_type')
    click_button "Add"
 
    expect(page).to have_content('Remove') if verify_button
 
    click_button "Save review questionnaire"
 
    expect(page).to have_content('All questions has been successfully saved!') if verify_button
  end
 
  def edit_created_question
    first("textarea[placeholder='Edit question content here']").set "Question edit"
    click_button "Save review questionnaire"
    expect(page).to have_content('All questions has been successfully saved!')
    expect(page).to have_content('Question edit')
  end
 
  def check_deleted_question
    click_on('Remove')
    expect(page).to have_content('You have successfully deleted the question!')
  end
 
  def choose_check_type command_type
    if command_type == 'edit'
      edit_created_question
    else
      check_deleted_question
    end
  end
 
  describe "Edit and delete a question" do
    question_type.each do |q_type|
      %w(edit delete).each do |q_command|
        it "is able to " + q_command + " " + q_type + " question" do
          load_question q_type, false
          choose_check_type q_command
        end
      end
    end
  end
 
'''Refactoring quiz_spec'''
The quiz_spec covers the testing of creation and use of quizes for instructors and students.  Similar to the other files which were refactored, this spec had several areas of duplicated code which were extracted and placed into individual definitions.  The interesting thing about updating this module was it's high ABC (Assignment, Branch, Condition) count.  In order to reduce this metric, several of the definitions needed to be split into logical methods to be called by the refactored methods.  Below shows and example where new definitions were made in order to reduce ABC score.
 
  def fill_in_choices
    # Fill in for all 4 choices
    fill_in 'new_choices_1_MultipleChoiceRadio_1_txt', with: 'Test Quiz 1'
    fill_in 'new_choices_1_MultipleChoiceRadio_2_txt', with: 'Test Quiz 2'
    fill_in 'new_choices_1_MultipleChoiceRadio_3_txt', with: 'Test Quiz 3'
    fill_in 'new_choices_1_MultipleChoiceRadio_4_txt', with: 'Test Quiz 4'
  end
 
  def fill_in_quiz
    # Fill in the form for Name
    fill_in 'questionnaire_name', with: 'Quiz for test'
    # Fill in the form for Question 1
    fill_in 'text_area', with: 'Test Question 1'
    # Choose the quiz to be a single choice question
    page.choose('question_type_1_type_multiplechoiceradio')
    fill_in_choices
    # Choose the first one to be the correct answer
    page.choose('new_choices_1_MultipleChoiceRadio_1_iscorrect_1')
    # Save quiz
    click_on 'Create Quiz'
  end

Latest revision as of 02:08, 1 April 2017

E1724 - Refactoring Feature Tests

About Expertiza

Expertiza is an open source web application project based on Ruby on Rails framework. It provides an online interactive platform for instructors to post and grade assignments, and for students to contribute to team-based projects as well as individual assignments.

Problem Statement

Remove duplicated code in feature tests and improve the overall Code Climate. Code Climate aides in determining the DRYness and style of code, more information can be found at https://codeclimate.com/dashboard.

Running Tests

After building the Expertiza environment run 'rspec spec/features/*_spec.rb' to run all of the feature tests. This will run the feature spec test files.

Test Plan

Since this is a refactoring of rspec tests, the testing will consist of running the tests as described in the previous section and ensuring that all of the tests pass. In order to fully test that the refactoring has worked the negative of the expect statements where tested to ensure that they failed and that the tests were still running as expected. Once the test failed as expected the tests were changed back to run to success.

Refactoring Delayed_mailer Method

Delayed_mailer_spec method covers testing scenarios with the email reminder feature targeting various users and tasks. This method took time stamps using time_parse function, which would create issues when users change their time zones. Calling time_zone_parse instead of time_parse solves the issue. This method also had massive duplicate code for different test scenarios. A helper method enqueue_delayed_job(stage) is used to encapsulate the task being performed.

After refactoring in delayed_mailer_spec.rb each of the test cases were written in less than five lines of code.

Refactoring Scheduled_task_spec Method

Scheduled_task_spec method covers testing scenarios with scheduling for the deadline reminder feature targeting various users and tasks. This method took time stamps using time_parse function, which would create issues when users change their time zones. Calling time_zone_parse instead of time_parse solves the issue. This method also had massive duplicate code for different test scenarios. A helper method enqueue_scheduled_tasks(stage) is used to encapsulate the task being performed.

After refactoring in scheduled_spec.rb the majority of the test cases were written in less than five lines, with the exception of the team formation test which required additional logic but was still significantly compressed.

Refactoring Assignment_creation_spec

Assignment_creation_spec covers testing scenarios that create public and private assignments as well as the various options in this assignment creation. Using CodeClimate it was identified that a large portion of the code was duplicated across multiple test cases which violates the DRY principle. The redundant code was generalized and placed in methods instead of being written in each of the test cases and redundant methods that were never called were removed from class.

The code below is a sample of the refactored code where instead of having redundant code, handle_questionaire is called with a few parameters and all of the redundant code in the test cases is replaced.

 def validate_attributes(questionaire_name)
   questionnaire = get_questionnaire(questionaire_name).first
   expect(questionnaire).to have_attributes(
     questionnaire_weight: 50,
     notification_limit: 50
   )
 end
 def validate_dropdown
   questionnaire = Questionnaire.where(name: "ReviewQuestionnaire2").first
   assignment_questionnaire = AssignmentQuestionnaire.where(assignment_id: @assignment.id, questionnaire_id: questionnaire.id).first
   expect(assignment_questionnaire.dropdown).to eq(false)
 end
 def fill_in_questionaire(questionaire_css, questionaire_name)
   within(:css, questionaire_css) do
     select questionaire_name, from: 'assignment_form[assignment_questionnaire][][questionnaire_id]'
     uncheck('dropdown')
     select "Scale", from: 'assignment_form[assignment_questionnaire][][dropdown]'
     fill_in 'assignment_form[assignment_questionnaire][][questionnaire_weight]', with: '50'
     fill_in 'assignment_form[assignment_questionnaire][][notification_limit]', with: '50'
   end
   click_button 'Save'
   sleep 1
 end
 def handle_questionaire(questionaire_css, questionaire_name, test_attributes)
   fill_in_questionaire(questionaire_css, questionaire_name)
   if test_attributes
     validate_attributes(questionaire_name)
   else
     validate_dropdown
   end
 end


Refactoring Instructor_interface_spec

Instructor_interface_spec covers testing scenarios like creating a course, importing tests, and viewing publishing rights. Unlike assignment_creation_spec, the largest violation (as determined by CodeClimate) of the DRY principle was functionality that was exactly duplicated in questionnaire_spec. In order to fix this /spec/helpers/instructor_interface_helper_spec was created as a module and then included in both of the other Ruby files as a mixin.


Refactoring questionnaire_spec

The questionnaire_spec covers testing scenarios relating to creating questionnaires and filling them out. The unique thing about the questionnaire_spec was that it had a lot of instances of similar code, but not identical. This is because there are many different types of questions and every variation has to be tested. Aside from the specific question name, the process of testing the editing and deleting of each question type was the same. In order to make this more generic and repeatable, a new method was created which took the question type as an input and an each operator was used to cycle through each type and to test editing and deletion. Below is the definition which was created to test each question type for the ability to edit and delete.

 question_type = %w(Criterion Scale Dropdown Checkbox TextArea TextField UploadFile SectionHeader TableHeader ColumnHeader)
 def load_question question_type, verify_button
   load_questionnaire
   fill_in('question_total_num', with: '1')
   select(question_type, from: 'question_type')
   click_button "Add"
 
   expect(page).to have_content('Remove') if verify_button
 
   click_button "Save review questionnaire"
 
   expect(page).to have_content('All questions has been successfully saved!') if verify_button
 end
 def edit_created_question
   first("textarea[placeholder='Edit question content here']").set "Question edit"
   click_button "Save review questionnaire"
   expect(page).to have_content('All questions has been successfully saved!')
   expect(page).to have_content('Question edit')
 end
 def check_deleted_question
   click_on('Remove')
   expect(page).to have_content('You have successfully deleted the question!')
 end
 def choose_check_type command_type
   if command_type == 'edit'
     edit_created_question
   else
     check_deleted_question
   end
 end
 describe "Edit and delete a question" do
   question_type.each do |q_type|
     %w(edit delete).each do |q_command|
       it "is able to " + q_command + " " + q_type + " question" do
         load_question q_type, false
         choose_check_type q_command
       end
     end
   end
 end

Refactoring quiz_spec The quiz_spec covers the testing of creation and use of quizes for instructors and students. Similar to the other files which were refactored, this spec had several areas of duplicated code which were extracted and placed into individual definitions. The interesting thing about updating this module was it's high ABC (Assignment, Branch, Condition) count. In order to reduce this metric, several of the definitions needed to be split into logical methods to be called by the refactored methods. Below shows and example where new definitions were made in order to reduce ABC score.

 def fill_in_choices
   # Fill in for all 4 choices
   fill_in 'new_choices_1_MultipleChoiceRadio_1_txt', with: 'Test Quiz 1'
   fill_in 'new_choices_1_MultipleChoiceRadio_2_txt', with: 'Test Quiz 2'
   fill_in 'new_choices_1_MultipleChoiceRadio_3_txt', with: 'Test Quiz 3'
   fill_in 'new_choices_1_MultipleChoiceRadio_4_txt', with: 'Test Quiz 4'
 end
 def fill_in_quiz
   # Fill in the form for Name
   fill_in 'questionnaire_name', with: 'Quiz for test'

   # Fill in the form for Question 1
   fill_in 'text_area', with: 'Test Question 1'

   # Choose the quiz to be a single choice question
   page.choose('question_type_1_type_multiplechoiceradio')

   fill_in_choices

   # Choose the first one to be the correct answer
   page.choose('new_choices_1_MultipleChoiceRadio_1_iscorrect_1')

   # Save quiz
   click_on 'Create Quiz'
 end