CSC/ECE 517 Spring 2017 E1711: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
Line 33: Line 33:
=Remove duplicated code between delayed_mailer.rb and scheduled_task.rb=
=Remove duplicated code between delayed_mailer.rb and scheduled_task.rb=


After a deep investigation into understanding why the same code would appear in two separate files, we concluded that the authors of <i>scheduled_task.rb</i> found that <i>delayed_mailer.rb</i> already integrated with a Ruby gem called <i>delayed_job</i>.  <i>delayed_job</i> is used to run tasks asynchronously in the background.  It was used in <i>delayed_mailer</i> to add an outgoing email to a queue to send out in the future.  Instead of recreating similar infrastructure, the authors copied over the file to a different location, added on the new desired fucntionality, and renamed it.  The authors also pointed all existing code to use <i>scheduled_task.rb</i> instead of <i>delayed_mailer.rb</i>
===Background of Objective===
 
After a deep investigation into understanding why the same code would appear in two separate files, we concluded that the authors of <i>scheduled_task.rb</i> found that <i>delayed_mailer.rb</i> already integrated with a Ruby gem called <i>delayed_job</i>.  <i>delayed_job</i> is used to run tasks asynchronously in the background.  It was used in <i>delayed_mailer</i> to add an outgoing email to a queue to send out in the future.  Instead of recreating similar infrastructure, the authors copied over the file to a different location, added on the new desired functionality, and renamed it.  The authors also pointed all existing code to use <i>scheduled_task.rb</i> instead of <i>delayed_mailer.rb</i>
 
===Existing Tests===
 
We did not find any existing automated testing targeting this area of the code.  All existing tests in the <code>spec</code> file associated with this feature were copies of the tests in the delayed mailer feature and did not exercise any of the new code that was introduced with adding the new types of scheduled tasks.
 
=== Our Testing===
 
We went back to the demonstration of this feature and manually tested all the same scenarios.  We verified that no functionality presented in the demonstration was broken.  As we did not anticipate that there will not be any testing

Revision as of 00:13, 23 March 2017

E1711. Refactor delayed_mailer.rb and scheduled_task.rb


Overview

Introduction to Expertiza

Expertiza is a web based tool that allows instructors to create collaborative assignments where students can provide peer feedback on each others work. It provides instructors a system to manage assignments and different courses.

Scope of project E1711

The goal of E1711 was to refactor code found in the files delayed_mailer.rb and scheduled_task.rb.

The following issues were identified as the scope of this project:


  • scheduled_task.rb duplicated most of its code from delayed_mailer.rb. Our objective is to reduce/remove the duplication in keeping with the DRY principle.
  • The perform method uses a giant case statement, instead we were to incorporate the use of polymorphism
  • The mail_signed_up_users is long and should be broken into smaller and better named methods
  • Add/modify test cases as we added/removed/modified areas of the code


Information about the assignment can be found on this document

Test Plan

As this was a purely refactoring effort, our testing consisted of confirming existing tests did not break, modifying existing tests to match our changes, or adding tests as needed. The below sections will cover each objective in detail and will include information on how testing was done for each of those changes.


Remove duplicated code between delayed_mailer.rb and scheduled_task.rb

Background of Objective

After a deep investigation into understanding why the same code would appear in two separate files, we concluded that the authors of scheduled_task.rb found that delayed_mailer.rb already integrated with a Ruby gem called delayed_job. delayed_job is used to run tasks asynchronously in the background. It was used in delayed_mailer to add an outgoing email to a queue to send out in the future. Instead of recreating similar infrastructure, the authors copied over the file to a different location, added on the new desired functionality, and renamed it. The authors also pointed all existing code to use scheduled_task.rb instead of delayed_mailer.rb

Existing Tests

We did not find any existing automated testing targeting this area of the code. All existing tests in the spec file associated with this feature were copies of the tests in the delayed mailer feature and did not exercise any of the new code that was introduced with adding the new types of scheduled tasks.

Our Testing

We went back to the demonstration of this feature and manually tested all the same scenarios. We verified that no functionality presented in the demonstration was broken. As we did not anticipate that there will not be any testing