CSC/ECE 517 Spring 2017/E1724: Difference between revisions
No edit summary |
No edit summary |
||
(11 intermediate revisions by 3 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''' | ||
Delayed_mailer_spec method covers testing scenarios with the email reminder feature targeting various users and tasks. | 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 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 | 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 | '''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 | 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 | ||
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 |
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