CSC/ECE 517 Spring 2014/final doc updated qyx: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
 
(55 intermediate revisions by 3 users not shown)
Line 4: Line 4:
== Introduction ==
== Introduction ==
This project will be an extension of our [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2014/oss_E1406_st previous E1406 project]. For the previous project, we did some refactoring and testing for the controllers related to teams. This time, we are going to reuse most of our test environment, but extend the tests to other team-related classes. We also expect to examine and report the code that may require refactoring.
This project will be an extension of our [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Spring_2014/oss_E1406_st previous E1406 project]. For the previous project, we did some refactoring and testing for the controllers related to teams. This time, we are going to reuse most of our test environment, but extend the tests to other team-related classes. We also expect to examine and report the code that may require refactoring.
== Test Environment ==
== Test Environment ==


Line 9: Line 10:
;Sample data : Fixtures (under /test/fixtures)
;Sample data : Fixtures (under /test/fixtures)
;Coverage tool: [https://rubygems.org/gems/simplecov SimpleCov]
;Coverage tool: [https://rubygems.org/gems/simplecov SimpleCov]
=== Statistics ===
''Please note that the coverage generate by SimpleCov may be lower than expect due to the added fixtures.''
; Overall (set of 6 tests) : 105 tests, 156 assertions, 2 failures, 3060 / 13457 LOC (22.74%) covered.
; Unit tests : 18 tests, 18 assertions, 0 failures
; Functional tests : 87 tests, 138 assertions, 2 failures (reasons of failures can be found in "Possible Bugs")
== Running Tests ==
Please note that all of our work will be under the /test folder, which means they will not show any change on the view.
Run
rake test:prepare
before each action.
To run unit tests
bundle exec ruby -I.:test -e "ARGV.each{|f| require f}" test/unit/team_test.rb test/unit/course_team_test.rb test/unit/assignment_team_test.rb test/unit/team_user_test.rb
To run functional tests
bundle exec ruby -I.:test -e "ARGV.each{|f| require f}" test/functional/advertise_for_partner_controller_test.rb test/functional/teams_users_controller_test.rb
You can also run multiple tests
  bundle exec ruby -I.:test -e "ARGV.each{|f| require f}" test/functional/teams_users_controller_test.rb test/functional/advertise_for_partner_controller_test.rb test/unit/course_team_test.rb test/unit/assignment_team_test.rb test/unit/team_test.rb test/unit/team_user_test.rb
running the whole test suite with <code>rake test</code> is not recommended, as the schema change overtime has broke some of the previous tests.


== Background ==
== Background ==
Line 24: Line 46:
* Both assignment_team class and course_team class are inherited from team class.
* Both assignment_team class and course_team class are inherited from team class.
* These two classes have similar methods, which could be used to manage teams for assignment and course more effectively.
* These two classes have similar methods, which could be used to manage teams for assignment and course more effectively.
==Classes==
== Summary of Found Issues ==
=== app/models/teams_users.rb===
=== Changes made to the class===
The teams_users.rb is a model belonging to both users and teams. It is used when a user is added to a team.
==== team.rb ====  
''Based on the discussion from the demo, the duplicated method was deleted instead of commented out''
* duplicated def get_participants was removed
** This is removed because the same method is defined again at line 20
* duplicated def self.generate_team_name() was removed
** This is removed because the same method is defined again at line 131
==== advertise_for_team_controller.rb ====
* def new was removed
** since it's empty, there is no need to keep the code there


=== app/controllers/teams_users_controller.rb===
=== Possible Refactoring ===
The teams_users_controller support team member management for instructor/TA accounts
==== team.rb ====  
Instruction/TA can:
* line 81 def self.create_node_object(name, parent_id)
* get a user name by inputting part of the name
** see the TODO at line 80, the method will not work.
* list team members of a team  
=== advertise_for_team_controller.rb ===
* add a user to a team
* ''update'' should be ''edit'' (this is mentioned in the comments of code)
* remove a team member from a team
* ''edit'' should be reexamined since it is only assigning the team object
* remove all members belonging to an item
=== assignment_team.rb ===
* in "delete" method, before sign_up.each.destory, we need to make sure sign_up is not nil.
* in "email" method, a method named get_team_users is called but this method has no definition. We should define get_team_users somewhere or delete "email" method.
=== course_team.rb ===
* add_participant(course_id, user)method should belong to course rather than course_team. Delete this method.
* self.export_all_assignment_team_related_to_course(csv, parent_id, options)method should be deleted.


=== app/controllers/advertise_for_partners_controller.rb ===
=== teams_user.rb ===
The advertise_for_partners_controller work with the student_team_controllers, allowing student manage advertisement for their teams.
* ''delete'' method should not call ''team.delete'', comment out corresponding statements.
* ''self.is_team_empty'' method should not call ''length'', replace corresponding statements with ''count'' method


A student can:
=== teams_users_controller.rb ===
* create a new advertisement for the team
* ''delete_selected'' method mistake using ''find'' and ''first'', this method could not work. No test will be made on the method
* edit the advertisement content
* remove advertisement record from the table for the specific team
 
=== app/models/team.rb ===
The “team” object represents a team that consists students who are either participants of course or assignment.
a team will response to the request on:
 
* get_participants : get participant in the team
* delete : delete a team member in the team
* get_node_type : will return “TeamNode”
* get_author_name : will return team’s name
* generate_team_name() : serialize the automatically generated team name (e.g. team 1, team 2, etc.)
* get_possible_team_members(name) : find a team member by name
* has_user(user : find a team member by user object
* add_member(user, assignment_id) : add a user to an assignment team
* copy_members(new_team) : copy current member in the team to the new team
* create_node_object(name, parent_id) : ‘’there is a TODO on this method indicated that this is not working’’
* check_for_exisiting(parent, name, team_type) : check if the team name under the same assignment or course is in use.
* delete_all_by_parent(parent) : delete all teams under the same course/assignment
* randomize_all_by_parent(parent, team_type, team_size) : randomly assign team members
* generate_team_name(teamnameprefix) : generate team name with prefix (e.g. prefix_Team1)
* import_team_members(starting_index, row) : import user from specific index of users and length


=== app/unit/assignment_team.rb ===
=== Possible Bugs ===
* includes?(participant): check if this assignment team include this participant
==== team.rb ====
* assign_reviewer(reviewer): assign a reviewer for this assignment team
* def add_member(user, assignment_id) allow nil to be passed
* reviewed_by?(reviewer):check if this assignment team is reviewed by this reviewer
** rails always raise runtime error when nil is passed. [[#unit/team_test.rb|Test: "add member for course should create teams user" ]]
* topic: get the topic picked up by this team
* def randomize_all_by_parent passing wrong arguments
* has_submissions?: check if this team submitted work or not
** The code is passing wrong arguments in line 124 for add_member, there should be 2 arguments. [[#unit/team_test.rb|Test: "randomize_all_by_parent" ]]
* reviewed_contributor?(contributor)
* def import_team_members passing wrong arguments
* participants: get all the participants
** it is calling add_member and passing nil for assignment_id, which will make rails raise error.[[#unit/team_test.rb|Test: "import team members successfully" ]]
* delete: delete the signed up users
==== assignment_team.rb ====
* self.get_first_member(team_id)
* def self.get_export_fields(options)
* get_hyperlinks
** when the option team_name => false, the second element of the output array should be "Team members". But from the test, it is "Assignment Name". This is one failure from assignment_team_test.rb
* get_path
* def add_participant(assignment_id, user)
* get_submitted_files
** the AssignmentParticipant.count does not increase. This is another failure from assignment_team_test.rb.
* get_review_map_type
==== teams_user.rb ====
* self.handle_duplicate(team, name, assignment_id, handle_duplicates)
* def delete(option)
* self.import(row,session,assignment_id,options)
** should not call ''team.delete'', it will cause recursion error
* email
* def self.is_team_empty(team_id)
* get_participant_type
** rewrite this method without using ''length'' method, the ''length'' method will generate errors here
* get_parent_model
==== teams_users_controller.rb ====
* fullname
* def delete_selected(options)
* get_participants
** the statement ''TeamsUser.find(item_id).first'' cannot work, since the method ''find'' will return an exact object, which cannot be applied ''first'' method
* copy(course_id)
* add_participant(assignment_id, user)
* assignment
* get_scores(questions)
* self.get_team(participant)
* self.export(csv, parent_id, options)
* self.get_export_fields(options)
* self.create_team_and_node(assignment_id)
* self.remove_team_by_id(id)


=== app/unit/course_team.rb ===
== Setup ==
== Setup ==


To set up our testing for the models and controllers, we need some fixtures to support.  
To set up our testing for the models and controllers, we need some fixtures to support.  
Fixtures needed for corresponding classes are listed below
Fixtures needed for corresponding classes are listed below
'''However, all of the fixtures are required to be in the fixutre folder, otherwise it will raise error.'''


=== fixtures use in common ===
=== fixtures use in common ===
Line 120: Line 125:
[[File:Usecase_E1414_advertise.jpg]]
[[File:Usecase_E1414_advertise.jpg]]


== Testing Scenarios ==
== Concerns for Test Cases ==


=== teams_users_controller.rb ===
=== functional/teams_users_controller_test.rb ===
[[File:TeamUsersStory.png]]
[[File:TeamUsersStory.png]]


Based on the user story of the teams_users_controller.rb, there are also several sub-scenarios for each method in the user story.
=== functional/teams_users_controller_test.rb ===
 
Because most of the tests are straightforward, only tests with concerns are listed.
* Auto_complete_for_user_name:
** Test auto_complete_for_user_name with valid user name
** Test auto_complete_for_user_name with invalid user name
 
* List
** Test List with valid team id
** Test List with invalid team id
 
* New
** Test New with valid team id
** Test New with invalid team id
 
* Create
** Test Create with valid team id
*** Test Create with valid user id and maximum member number is not reached
*** Test Create with invalid user id and maximum member number is not reached
*** Test Create with valid user id and maximum member number is reached
*** Test Create with invalid user id and maximum member number is reached
** Test Create with invalid team id
*** Test Create with valid user id and maximum member number is not reached
*** Test Create with invalid user id and maximum member number is not reached
*** Test Create with valid user id and maximum member number is reached
*** Test Create with invalid user id and maximum member number is reached
 
* Delete
** Test Delete with valid user id
** Test Delete with invalid user id
 
* Delete_select
* Delete_select
** Test Delete_select with valid item id
** Please notice that this method will receive an ''item_id'' and delete the first team user belonging to that item
** Test Delete_select with invalid item id
 
=== teams_users.rb ===


=== unit/teams_users_test.rb ===
Only tests with concerns are listed.
* Delete
* Delete
** Test Delete to check if it could delete the team node and the team user
** Notice here we would not test if the method will delete team, only test if it could delete the team user
 
* Remove_team
** Test Remove_team with valid user_id and valid team_id
** Test Remove_team with valid user_id and invalid team_id
** Test Remove_team with invalid user_id and valid team_id
** Test Remove_team with invalid user_id and invalid team_id


* Is_team_empty
=== functional/advertise_for_partner_controller_test.rb ===
** Test Is_team_empty with empty team
* # test "test update advertisement fail" do
** Test Is_team_empty with non-empty team
** should show flash for fail message when team.save fail, but there is no really a good way to fail team.save, so this is not implemented


* Add_member_to_invited_team
=== unit/team_test.rb ===
** Test Add_member_to_invited_team with valid user_id and valid team_id
Most of test should be straightforward, so only the test with concerns will be listed below
** Test Add_member_to_invited_team with valid user_id and invalid team_id
* test "get_participants from assignment team" do
** Test Add_member_to_invited_team with invalid user_id and valid team_id
** Please notice that this will actually call the get_participants under AssignmentTeam
** Test Add_member_to_invited_team with invalid user_id and invalid team_id


=== advertise_for_partner_controller.rb ===
* test "generate_team_name" do
* remove
** Please notice that this method (self.generate_team_name)is modified
** test remove with valid user id
** test remove with invalid user id


* create
* test "add member for course should create teams user" do
** test create with valid user id
** In the original code, it allow nil to be passed as parameter, and checks for assignment_id = nil, but rails will actually raise runtime error when nil is passed.
** test create with invalid user id


* update
* #test "randomize_all_by_parent" do
** test update with invalid id
** Commented out because the bug in the code make it raise error because assignment id not passed. (wrong argument)
** test update success
** test update fail (should remain the same)


* edit
* #test "import team members successfully" do
** test edit
** problem: add_member are not passing correct assignment id in team.rb
=== team.rb ===
* get participants
** test get participants


=== unit/assignment_team_test.rb ===
Because most of the tests are straightforward, only tests with concerns are listed.
* delete
* delete
** test delete with team node
** this method has bug. In the method, before sign_up.each.destory, we need to make sure sign_up is not nil. No test
** test delete without team node
* email
 
** this method has bug because there is no method named get_team_users. No test
* get_node_type
** test get node type
 
* get_author_name
** test author name
 
* generate_team_name()
** test generate team name from zero
** test generate team name from one
 
* get_possible_team_members(name)
** test get possible team members by exact name
** test get possible team members by similar name
** test get possible team members by name in other team
 
* has_user(user)
** test has user with valid user
** test has user with invalid user
 
* add_member(user, assignment_id)
** test add member with valid user
** test add member already in team
** test add member without assignment id
** test add member when max team size reached
 
* copy_members(new_team)
** test copy members to empty new team
** test copy members to nonempty new team
** test copy members with no member in current team
 
* create_node_object(name, parent_id)
** ‘’there is a TODO on this method indicated that this is not working’’
 
* check_for_exisiting(parent, name, team_type)
** test check for exisiting with exisited name
** test check for exisiting with nonexisted name
 
* delete_all_by_parent(parent)
** test delete with valid parents
** test delete with invalid parents
 
* randomize_all_by_parent(parent, team_type, team_size)
** test randomize all by parent with exact number
** test randomize all by parent without enough people
** test randomize all by parent with remainder
 
* generate_team_name(teamnameprefix)
** test generate prefix team name with valid name
** test generate prefix team name with duplicated name
 
* import_team_members(starting_index, row)
** test import team members with valid index and row
** test import team members with invalid index and row
 
== Possible Refacotring ==
=== advertise_for_team_controller.rb ===
* ''update'' should be ''edit'' (this is mentioned in the comments of code)
* ''edit'' should be reexamined since it is only retrieving the team object
* ''new'' is empty, so it should be removed
 
=== team.rb ===
* There are two ''def get_participants'', the second one overwritten the first one, so the first one should be deleted.


== Running Tests ==
=== unit/course_team_test.rb ===
Please note that all of our work will be under the /test folder, which means they will not show any change on the view.  
Only tests with concerns are listed.
To see the result of tests, run
*add_participant(course_id, user)
rake test TEST=test/unit/<filename> # for models
** this method should belong to course rather than course_team. This test may be not useful.
and
* self.export_all_assignment_team_related_to_course(csv, parent_id, options)
rake test TEST=test/functional/<filename> # for controllers
** this method should be deleted from course_team class. No test


running the whole test suite with <code>rake test</code> is not recommended, as the schema change overtime has broke some of the previous tests.
== Possible Refactoring ==
=== teams_participant.rb & teams_participants_controller.rb ===
* These two files are never used, no tests will cover them.


==Sample Test Code==
==Sample Test Code==

Latest revision as of 17:25, 29 April 2014

E1414: Testing classes related to teams

Please be advised that we do not have deployment link, since our work are all back-end tests

Introduction

This project will be an extension of our previous E1406 project. For the previous project, we did some refactoring and testing for the controllers related to teams. This time, we are going to reuse most of our test environment, but extend the tests to other team-related classes. We also expect to examine and report the code that may require refactoring.

Test Environment

Framework
Rails default test framework
Sample data
Fixtures (under /test/fixtures)
Coverage tool
SimpleCov

Statistics

Please note that the coverage generate by SimpleCov may be lower than expect due to the added fixtures.

Overall (set of 6 tests)
105 tests, 156 assertions, 2 failures, 3060 / 13457 LOC (22.74%) covered.
Unit tests
18 tests, 18 assertions, 0 failures
Functional tests
87 tests, 138 assertions, 2 failures (reasons of failures can be found in "Possible Bugs")

Running Tests

Please note that all of our work will be under the /test folder, which means they will not show any change on the view. Run

rake test:prepare

before each action. To run unit tests

bundle exec ruby -I.:test -e "ARGV.each{|f| require f}" test/unit/team_test.rb test/unit/course_team_test.rb test/unit/assignment_team_test.rb test/unit/team_user_test.rb

To run functional tests

bundle exec ruby -I.:test -e "ARGV.each{|f| require f}" test/functional/advertise_for_partner_controller_test.rb test/functional/teams_users_controller_test.rb

You can also run multiple tests

 bundle exec ruby -I.:test -e "ARGV.each{|f| require f}" test/functional/teams_users_controller_test.rb test/functional/advertise_for_partner_controller_test.rb test/unit/course_team_test.rb test/unit/assignment_team_test.rb test/unit/team_test.rb test/unit/team_user_test.rb

running the whole test suite with rake test is not recommended, as the schema change overtime has broke some of the previous tests.

Background

teams_users.rb & teams_users_controller.rb

  • Teams_users.rb and teams_users_controller.rb are used for Instructor/TA to manage the members of teams.
  • There is no functional test for teams_users_controller.rb and unit test for teams_users.rb in the current version of expertiza codes.

  • This controller takes over the advertisement function from the student_team_controller, and make the class more RESTful.
  • It seems like there has been a functional test, but the class name and set up looks like the other class. It may be necessary to re-run, and even re-write the test

team.rb

  • This model defines the relationship between team object and other objects. It also has other methods to handle related functions about team operation
  • There exists a unit test for team model, but there are only two test cases. Also, some of the methods under team.rb are duplicated, or either marked as not working. Further research may be required before starting to do the test.

assignment_team.rb & course_team.rb

  • Both assignment_team class and course_team class are inherited from team class.
  • These two classes have similar methods, which could be used to manage teams for assignment and course more effectively.

Summary of Found Issues

Changes made to the class

team.rb

Based on the discussion from the demo, the duplicated method was deleted instead of commented out

  • duplicated def get_participants was removed
    • This is removed because the same method is defined again at line 20
  • duplicated def self.generate_team_name() was removed
    • This is removed because the same method is defined again at line 131

  • def new was removed
    • since it's empty, there is no need to keep the code there

Possible Refactoring

team.rb

  • line 81 def self.create_node_object(name, parent_id)
    • see the TODO at line 80, the method will not work.

  • update should be edit (this is mentioned in the comments of code)
  • edit should be reexamined since it is only assigning the team object

assignment_team.rb

  • in "delete" method, before sign_up.each.destory, we need to make sure sign_up is not nil.
  • in "email" method, a method named get_team_users is called but this method has no definition. We should define get_team_users somewhere or delete "email" method.

course_team.rb

  • add_participant(course_id, user)method should belong to course rather than course_team. Delete this method.
  • self.export_all_assignment_team_related_to_course(csv, parent_id, options)method should be deleted.

teams_user.rb

  • delete method should not call team.delete, comment out corresponding statements.
  • self.is_team_empty method should not call length, replace corresponding statements with count method

teams_users_controller.rb

  • delete_selected method mistake using find and first, this method could not work. No test will be made on the method

Possible Bugs

team.rb

assignment_team.rb

  • def self.get_export_fields(options)
    • when the option team_name => false, the second element of the output array should be "Team members". But from the test, it is "Assignment Name". This is one failure from assignment_team_test.rb
  • def add_participant(assignment_id, user)
    • the AssignmentParticipant.count does not increase. This is another failure from assignment_team_test.rb.

teams_user.rb

  • def delete(option)
    • should not call team.delete, it will cause recursion error
  • def self.is_team_empty(team_id)
    • rewrite this method without using length method, the length method will generate errors here

teams_users_controller.rb

  • def delete_selected(options)
    • the statement TeamsUser.find(item_id).first cannot work, since the method find will return an exact object, which cannot be applied first method

Setup

To set up our testing for the models and controllers, we need some fixtures to support. Fixtures needed for corresponding classes are listed below However, all of the fixtures are required to be in the fixutre folder, otherwise it will raise error.

fixtures use in common

  • teams.yml
  • users.yml
  • teams_users.yml
  • courses.yml
  • assignments.yml
  • teams_participant.yml
  • join_team_requests.yml
  • participants.yml
  • nodes.yml
  • questions.yml
  • signed_up_users.yml

Use Case Diagram

Concerns for Test Cases

functional/teams_users_controller_test.rb

functional/teams_users_controller_test.rb

Because most of the tests are straightforward, only tests with concerns are listed.

  • Delete_select
    • Please notice that this method will receive an item_id and delete the first team user belonging to that item

unit/teams_users_test.rb

Only tests with concerns are listed.

  • Delete
    • Notice here we would not test if the method will delete team, only test if it could delete the team user

functional/advertise_for_partner_controller_test.rb

  • # test "test update advertisement fail" do
    • should show flash for fail message when team.save fail, but there is no really a good way to fail team.save, so this is not implemented

unit/team_test.rb

Most of test should be straightforward, so only the test with concerns will be listed below

  • test "get_participants from assignment team" do
    • Please notice that this will actually call the get_participants under AssignmentTeam
  • test "generate_team_name" do
    • Please notice that this method (self.generate_team_name)is modified
  • test "add member for course should create teams user" do
    • In the original code, it allow nil to be passed as parameter, and checks for assignment_id = nil, but rails will actually raise runtime error when nil is passed.
  • #test "randomize_all_by_parent" do
    • Commented out because the bug in the code make it raise error because assignment id not passed. (wrong argument)
  • #test "import team members successfully" do
    • problem: add_member are not passing correct assignment id in team.rb

unit/assignment_team_test.rb

Because most of the tests are straightforward, only tests with concerns are listed.

  • delete
    • this method has bug. In the method, before sign_up.each.destory, we need to make sure sign_up is not nil. No test
  • email
    • this method has bug because there is no method named get_team_users. No test

unit/course_team_test.rb

Only tests with concerns are listed.

  • add_participant(course_id, user)
    • this method should belong to course rather than course_team. This test may be not useful.
  • self.export_all_assignment_team_related_to_course(csv, parent_id, options)
    • this method should be deleted from course_team class. No test

Possible Refactoring

teams_participant.rb & teams_participants_controller.rb

  • These two files are never used, no tests will cover them.

Sample Test Code

We show two pieces of test code for two controllers from our previous work to explain how our test works

   test "create_student team with name in use" do
       sessionVars = session_for(users(:student8))
       post(:create, {'team' => { 'name' => 'IntelligentTeam2'}, 'id' => participants(:par21).id, "commit" => "Create Team"}, sessionVars, nil)
       assert_equal 'Team name is already in use.', flash[:notice]
       assert_redirected_to :controller => 'student_team', :action => 'view', :id => participants(:par21).id
   end
   test "create_should_increase_number_of_teams_course" do
       sessionVars = session_for(users(:instructor1))
       sessionVars[:team_type] = "Course"
       assert_difference 'Team.count' do
         get :create, {'id' => @testCourse,'team' => {'name' => "Random"}},sessionVars
       end
   end

The first piece of code is from student_team_controller.rb. It tests if there is already a same team name when student8 want to create a team. The second piece of code is from teams_controller_test.rb. It tests if the team number decreases when instructor1 creates a new team for a course.

As we can see from the code, when we want to use post or get, several parameters are needed. Usually we use four parameters: First, the method we are requesting. Second, an hash of request parameters for the method. Third, an optional hash of session variables. Forth, an optional hash of flash values. If we do not want to pass in the optional hash, we can set it to nil as the first piece of code shows, or we can just leave it blank as the second piece of code shows.

When we run the command rake test TEST=test/functional/your_test_file.rb, you will see if the test is passed or not.

References

<references/>