CSC/ECE 517 Fall 2016/E1634.Refactor and write unit test of due date.rb and deadline helper.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
(Blanked the page)
Line 1: Line 1:
== '''Introduction''' ==


=== Expertiza ===
Expertiza<ref>{{Cite web|url=http://www.expertiza.ncsu.edu|access-date=2016-10-28}}</ref> is an open source project for school assignment management for instructors and students based on the Ruby on Rails<ref>{{Cite web|url=http://www.rubyonrails.org|access-date=2016-10-28}}</ref> framework. Expertiza allows the instructor to create new assignments and customise new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and word documents. Expertiza provides a dashboard for all the assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistants. In addition to assignments, it encompasses peer reviews wherein participants are allowed to provide feedback anonymously about each other's work thereby providing scope for the better outcome. The due_date.rb file is responsible for informing the users about the deadline for submission of the each assignment. Due dates in Expertiza have their association with many other components like assignments, reviews etc.
=== Code Climate ===
Code Climate<ref>{{Cite web|url=https://codeclimate.com/dashboard|title=Log in to your Code Climate dashboard with GitHub or your email.|website=codeclimate.com|access-date=2016-10-28}}</ref> is a tool that runs static analysis on a GitHub project and outputs many details like test coverage, complexity, duplication, security, style, and more. There is a Google Chrome extension to integrate the Code Climate results generated directly into GitHub which is visible when browsing the repository on the browser. It allows us to see issues displayed directly inside GitHub's UI, to review which lines are covered in diffs and files, and add repositories and open tickets without changing your workflow.
== '''Tasks Identified''' ==
To install Code climate Chrome Extension that highlights the duplicated code.
To refactor the following two files:
* Due_date.rb
** Ternary operators must not be nested. Prefer `if` or `else` constructs instead.
** Useless assignment to variable - `sorted_deadlines`.
** Prefer `each` over `for`.
** Use `find_by` instead of `where.first`.
** Correct the use of Time function
* Deadline_helper.rb
** Do not use `Time.now` without zone. Use one of `Time.zone.now`, `Time.current`, `Time.now.in_time_zone`, `Time.now.utc`, `Time.now.getlocal`, `Time.now.iso8601`, `Time.now.jisx0301`, `Time.now.rfc3339`, `Time.now.to_i`, `Time.now.to_f` instead.
** Trailing whitespace detected.
** Extra empty line detected at module body end.
* Create respective RSpec<ref>{{Cite web|url=http://www.rspec.info|access-date=2016-10-28}}</ref>files in /spec/models/ and /spec/helper folder and write unit tests for each method in due_date.rb and deadline_helper.rb.
== '''Current Implementation''' ==
DueDate is a Model class to manage the deadlines of an assignment. It has methods for setting due dates for an assignment, copying due dates from one assignment to a new assignment etc. DeadlineHelper provides helper functions that help DueDate perform certain tasks. The assignment focuses on refactoring some of the methods based on warnings received from Code Climate's static analysis and modifying the language to make it more Ruby-friendly. The assignment also involves writing unit test cases for due_date.rb and deadline_helper.rb in order to increase test coverage.
The goal of this project is to attempt to make this part of the application easier to read and write unit test cases that the application must pass.
== '''Changed Implementation''' ==
Changes implemented involves refactoring the code and making it more understandable by adding comments in the code.
The modified files are
* Due_date.rb (path: /app/models)
* Deadline_helper.rb (path: /app/helpers)
Testing files
* Due_date_spec.rb (path: /spec/models)
* Deadline_helper_spec.rb (path: /spec/helpers)
=== Due_date.rb ===
* Converted for..in loop to object.each in order to follow better Ruby syntax.
* Unnecessary assignment to sorted_deadlines removed.
* Nested ternary operators have been changed to if..else in order to make it more readable.
[[File:Duedaterb.png]]
* Changed where(...).first to find_by(...) which is the newer recommended syntax.
* Corrected the Time.now functions by adding the correct zones to them such as Time.zone.now.
* Removed trailing whitespaces.
[[File:DueDaterb2.png]]
=== Deadline_helper.rb ===
* Time functions were changed to functions with zones
* Extra line removed
[[File:Deadlinehelper.png]]
== '''RSpec testing''' ==
There were no existing tests for the functions in due_date.rb and deadline_helper.rb. We have added exhaustive set of RSpec tests to test all the code. We have added two new spec files 'due_date_spec.rb' and ‘deadline_helper_spec.rb’ which cover the testing scenarios for the functions in ‘due_date.rb’ and ‘deadline_helper.rb’.For both of these two files, all Travis CI<ref>{{Cite web|url=http://http://www.travis-ci.com|access-date=2016-10-28}}</ref> test cases have passed.
These RSpec files have 100% code coverage visible at: /coverage/index.html
=== due_date_spec.rb ===
This file is located at spec/models and tests the functionalities of the due_date.rb file located in app/models. There are 18 test cases in total which are listed below.
* If the factory is successfully able to build the due_date objects.<syntaxhighlight lang="ruby">
it "due date factory created successfully" do
    expect(@assignment_due_date).to be_valid
end
</syntaxhighlight><syntaxhighlight lang="ruby">
it "due dates created correctly" do
    expect(@due_dates.length).to be == 10
end
</syntaxhighlight>
* If the function "set_flag" successfully sets the due_date flag.<syntaxhighlight lang="ruby">
it "due date flag is set" do
    expect(@assignment_due_date.flag).to be false
    @assignment_due_date.set_flag
    expect(@assignment_due_date.flag).to be true
end
</syntaxhighlight>
* If the function "due_at_is_valid_datetime" returns nil (no errors) for a valid datetime in due_at (no invalid test cases can be added here because model does not allow invalid datetime to be set at all).<syntaxhighlight lang="ruby">
it "due at is valid datetime" do
    expect(@assignment_due_date.due_at_is_valid_datetime).to be nil
end
</syntaxhighlight>
* If the function "self.copy" is able to copy due dates from one assignment to another.<syntaxhighlight lang="ruby">
it "copy due dates to new assignment" do
    new_assignment_id = build(:assignment, id: 999).id
    old_assignment_id = @assignment_due_date.assignment.id
    DueDate.copy(old_assignment_id, new_assignment_id)
    expect(DueDate.where(parent_id: new_assignment_id).count).to eql DueDate.where(parent_id: old_assignment_id).count
end
</syntaxhighlight>
* If the function "self.set_duedate" is able to create another due date by copying data from an existing due date object.<syntaxhighlight lang="ruby">
it "create new duedate record with values" do
    DueDate.set_duedate({id: 999}, @assignment_due_date.deadline_type_id,
                        @assignment_due_date.parent_id, @assignment_due_date.round)
    new_due_date = DueDate.find_by(id: 999)
    expect(new_due_date).to be_valid
    expect(new_due_date.deadline_type_id).to eql @assignment_due_date.deadline_type_id
    expect(new_due_date.parent_id).to eql @assignment_due_date.parent_id
    expect(new_due_date.round).to eql @assignment_due_date.round
end
</syntaxhighlight>
* If the function "self.deadline_sort" is able to sort the due dates in ascending order.<syntaxhighlight lang="ruby">
it "sort duedate records" do
    sorted_due_dates = @due_dates
    expect(sorted_due_dates.each_cons(2).all?{|m1, m2| (m1.due_at <=> m2.due_at) != 1}).to eql false
    sorted_due_dates = DueDate.deadline_sort(@due_dates)
    expect(sorted_due_dates.each_cons(2).all?{|m1, m2| (m1.due_at <=> m2.due_at) != 1}).to eql true
end
</syntaxhighlight>
* If the function "self.done_in_assignment_round" returns the correct number of rounds for specific inputs. This involves an invalid test case as well for 0 rounds.<syntaxhighlight lang="ruby">
describe "#done_in_assignment_round" do
    it "return 0 when no response map" do
      response = ReviewResponseMap.create
      response.type = "ResponseMap"
      response.save
      expect(DueDate.done_in_assignment_round(1, response)).to eql 0
    end
    it "return round 1 for single round" do
      response = ReviewResponseMap.create
      expect(DueDate.done_in_assignment_round(@assignment_due_date.parent_id, response)).to eql 1
    end
end
</syntaxhighlight>
* If the function "self.get_next_due_date" works as expected. This involves several invalid test cases as well.<syntaxhighlight lang="ruby">
describe "#get_next_due_date" do
    it "no subsequent due date" do
      expect(DueDate.get_next_due_date(@assignment_due_date.parent_id)).to be nil
    end
    it "nil value throws exception" do
      expect { DueDate.get_next_due_date(nil) }.to raise_exception(ActiveRecord::RecordNotFound)
    end
    it "get next assignment due date" do
      due_date = create(:assignment_due_date, deadline_type: @deadline_type,
        submission_allowed_id: @deadline_right.id, review_allowed_id: @deadline_right.id,
        review_of_review_allowed_id: @deadline_right.id, due_at: Time.zone.now + 5000)
      expect(DueDate.get_next_due_date(due_date.parent_id)).to be_valid
    end
    it "get next due date from topic for staggered deadline" do
      assignment_id = create(:assignment, staggered_deadline: true, name: "testassignment").id
      due_date = create(:topic_due_date, deadline_type: @deadline_type,
        submission_allowed_id: @deadline_right.id, review_allowed_id: @deadline_right.id,
        review_of_review_allowed_id: @deadline_right.id, due_at: Time.zone.now + 5000, parent_id: assignment_id)
      expect(DueDate.get_next_due_date(assignment_id, due_date.parent_id)).to be_valid
    end
    it "next due date does not exist for staggered deadline" do
      assignment_id = create(:assignment, staggered_deadline: true, name: "testassignment").id
      due_date = create(:topic_due_date, deadline_type: @deadline_type,
        submission_allowed_id: @deadline_right, review_allowed_id: @deadline_right,
        review_of_review_allowed_id: @deadline_right, due_at: Time.zone.now + 5000, parent_id: assignment_id)
      expect(DueDate.get_next_due_date(assignment_id)).to be nil
    end
    it "next due date is before Time.now for staggered deadline" do
      assignment_id = create(:assignment, staggered_deadline: true, name: "testassignment").id
      due_date = create(:topic_due_date, deadline_type: @deadline_type,
        submission_allowed_id: @deadline_right, review_allowed_id: @deadline_right,
        review_of_review_allowed_id: @deadline_right, due_at: Time.zone.now - 5000, parent_id: assignment_id)
      expect(DueDate.get_next_due_date(assignment_id, due_date.parent_id)).to be nil
    end
    it "get next due date from assignment for staggered deadline" do
      assignment_id = create(:assignment, staggered_deadline: true, name: "testassignment").id
      due_date = create(:assignment_due_date, deadline_type: @deadline_type,
        submission_allowed_id: @deadline_right, review_allowed_id: @deadline_right,
        review_of_review_allowed_id: @deadline_right, due_at: Time.zone.now + 5000, parent_id: assignment_id)
      expect(DueDate.get_next_due_date(assignment_id)).to be_valid
    end
end
</syntaxhighlight>
* If the function "self.default_permission" returns the correct default permissions for particular deadline and permission types.<syntaxhighlight lang="ruby">
it "metareview review_of_review_allowed default permission OK" do
    expect(DueDate.default_permission('metareview', 'review_of_review_allowed')).to be == DeadlineRight::OK
end
</syntaxhighlight><syntaxhighlight lang="ruby">
it "review submission_allowed default permission NO" do
    expect(DueDate.default_permission('review', 'submission_allowed')).to be == DeadlineRight::NO
end
</syntaxhighlight>
To test this file run the following command:<syntaxhighlight>
rspec spec/models/due_date_spec.rb
</syntaxhighlight>
The output of this RSpec file is present in below screenshot:
[[File:Duedaterspec.png]]
Code coverage details of the above RSpec files is present in below screenshot:
[[File:Duedatecoverage.png]]
Code coverage details of the above RSpec files is present in below screenshot:
=== deadline_helper_spec.rb ===
This is a test file for testing the functionalities of Deadline_helper.rb file located at app/helpers. Different test cases present in this file are:
* Check if the factory<ref>{{Cite web|url=http://http://www.semaphoreci.com|access-date=2016-10-28}}</ref> is valid: 
<syntaxhighlight lang="ruby">
it "has a valid factory" do
    factory = FactoryGirl.build(:topic_due_date)
    expect(factory).to be_valid
end
</syntaxhighlight>
* Fail if the due date is invalid:
<syntaxhighlight lang="ruby">
it "should fail because of invalid due_date" do
      expect { DeadlineHelper.create_topic_deadline(nil, 0, 0)}.to raise_exception(NoMethodError)
end
</syntaxhighlight>
* If new due_date object is created:<syntaxhighlight lang="ruby">
it "new due_date object created" do
      DeadlineHelper.create_topic_deadline(@topic_due_date, 0, 1)
      expect(TopicDueDate.count).to be == 2
end
</syntaxhighlight>
* due_at should be same for 0 offset:<syntaxhighlight lang="ruby">
it "due_at should be same for 0 offset" do
      DeadlineHelper.create_topic_deadline(@topic_due_date, 0, 10)
      new_due_date = TopicDueDate.find_by(parent_id: 10)
      expect(new_due_date).to be_valid
      expect(new_due_date.due_at.to_s).to be == @topic_due_date.due_at.to_s
end
</syntaxhighlight>
* due_at is calculated correctly if offset is positive:<syntaxhighlight lang="ruby">
it "due_at calculated correctly for positive offset" do
      DeadlineHelper.create_topic_deadline(@topic_due_date, 5000, 10)
      new_due_date = TopicDueDate.find_by(parent_id: 10)
      expect(new_due_date).to be_valid
      expect(new_due_date.due_at.to_s).to be == (Time.zone.parse(@topic_due_date.due_at.to_s) + 5000).to_s
end
</syntaxhighlight>
* due_at is calculated correctly if offset is negative:<syntaxhighlight lang="ruby">
it "due_at calculated correctly for negative offset" do
      DeadlineHelper.create_topic_deadline(@topic_due_date, -5000, 10)
      new_due_date = TopicDueDate.find_by(parent_id: 10)
      expect(new_due_date).to be_valid
      expect(new_due_date.due_at.to_s).to be == (Time.zone.parse(@topic_due_date.due_at.to_s) - 5000).to_s
end
</syntaxhighlight>[[asd]]
* The offset is being converted to integer properly:<syntaxhighlight lang="ruby">
it "offset converted to integer correctly" do
      DeadlineHelper.create_topic_deadline(@topic_due_date, 5000.15, 10)
      new_due_date = TopicDueDate.find_by(parent_id: 10)
      expect(new_due_date).to be_valid
      expect(new_due_date.due_at.to_s).to be == (Time.zone.parse(@topic_due_date.due_at.to_s) + 5000).to_s
end
</syntaxhighlight>
To test this file run the following command:<syntaxhighlight>
rspec spec/models/deadline_helper_spec.rb
</syntaxhighlight>The output of this RSpec file is present in below screenshot:
[[File:Deadlinerspec.png]]
Code coverage details of the above RSpec files is present in below screenshot:
[[File:Deadlinecoverage.png]]
== '''References''' ==
"[http://www.expertiza.ncsu.edu]"
"[Ruby on Rails]". [http://www.rubyonrails.org]. Retrieved 2016-10-28.
"[Log in to your Code Climate dashboard with GitHub or your email].". [http://www.codeclimate.com]. Retrieved 2016-10-28.
"[RSpec: Behaviour Driven Development for Ruby]". [http://www.rspec.info]. Retrieved 2016-10-28.
"[Travis CI - Test and Deploy with Confidence]". [http://www.travis-ci.com link title]. Retrieved 2016-10-28
"[Working Effectively with Data Factories Using FactoryGirl]". [http://www.semaphoreci.com]. Retrieved 2016-10-28

Revision as of 23:53, 28 October 2016