User talk:Tcao
E1949. Write Unit Tests for Importing assignment participants and import glitches
This page is a description of Expertiza OSS project E.1949 Write Unit Tests for Importing assignment participants and import glitches.
Expertiza Introduction
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.
Problem Background
The import feature is the most helpful feature for instructors to set up assignments. The instructors usually have a list of students, teams, etc from their learning management system. Being able to import these into expertiza saves a lot of time when setting up an assignment.
Problem Statement
- Importing participants redirects to a confirmation screen listing the users that are going to be imported but the users do not seem to be imported.
- This is an intermittent issue that can be addressed through a test case.
Task 1. Write a test case to import participants.
We looked into the files responsible for importing assignment participants. As we understand, the imported file goes through the following process.
- Add one line to expect that #create_new_user method in import_file_helper is NOT called in each test case that the user HAS an account, and do the opposite in case the user DOES NOT HAVE an account.
Add specs to ensure create_new_user is called only if the user does not have an account
#spec/models/assignment_particpant_spec.rb context 'when the record has less than 4 items' do it 'raises an ArgumentError' do row = {name: 'no one', fullname: 'no one', email: 'no_one@email.com'} expect(ImportFileHelper).not_to receive(:create_new_user) expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error('The record containing no one does not have enough items.') end end context 'when certain assignment cannot be found' do it 'creates a new user based on import information and raises an ImportError' do allow(Assignment).to receive(:find).with(1).and_return(nil) expect(ImportFileHelper).to receive(:create_new_user) expect { AssignmentParticipant.import(row, nil, {}, 1) }.to raise_error('The assignment with id "1" was not found.') end end allow(AssignmentParticipant).to receive(:exists?).with(user_id: 1, parent_id: 1).and_return(false) allow(AssignmentParticipant).to receive(:create).with(user_id: 1, parent_id: 1).and_return(participant) allow(participant).to receive(:set_handle).and_return('handle') expect(ImportFileHelper).to receive(:create_new_user) expect(AssignmentParticipant.import(row, nil, {}, 1)).to be_truthy end end end
Fix bugs in assignment_particpant_spec
#spec/models/assignment_particpant_spec.rb context 'when the record has less than 4 items' do it 'raises an ArgumentError' do row = {name: 'no one', fullname: 'no one', email: 'no_one@email.com'} expect(ImportFileHelper).not_to receive(:create_new_user) expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error('The record containing no one does not have enough items.') end end context 'when certain assignment cannot be found' do it 'creates a new user based on import information and raises an ImportError' do allow(Assignment).to receive(:find).with(1).and_return(nil) expect(ImportFileHelper).to receive(:create_new_user) expect { AssignmentParticipant.import(row, nil, {}, 1) }.to raise_error('The assignment with id "1" was not found.') end end allow(AssignmentParticipant).to receive(:exists?).with(user_id: 1, parent_id: 1).and_return(false) allow(AssignmentParticipant).to receive(:create).with(user_id: 1, parent_id: 1).and_return(participant) allow(participant).to receive(:set_handle).and_return('handle') expect(ImportFileHelper).to receive(:create_new_user) expect(AssignmentParticipant.import(row, nil, {}, 1)).to be_truthy end end end
Correct spelling in the file name
spec/models/assignment_particpant_spec.rb → spec/models/assignment_participant_spec.rb
Task 2. Write a test case to import topics with special cases.
- Thoroughly test the #import method in the sign_up_topic model. This includes the following test cases:
- The record is empty
- The record is not empty and the topic is not existing, including special cases
- The record is not empty and the topic is existing, including special cases
Add tests for SignUpTopic
#spec/models/sign_up_topic_spec.rb describe SignUpTopic do let(:topic) { build(:topic) } describe ".import" do context 'when record is empty' do it 'raises an ArgumentError' do expect { SignUpTopic.import({}, nil, nil) }.to raise_error(ArgumentError, 'The CSV File expects the format: Topic identifier, Topic name, Max choosers, Topic Category (optional), Topic Description (Optional), Topic Link (optional).') end end context 'when the topic is not found' do it 'creates a new sign up topic' do row = {topic_name: 'name'} session = {assignment_id: 1} attributes = {topic_identifier: "identifier", topic_name: "name", max_choosers: "chooser", category: "category", description: "description", link: "link"} allow(SignUpTopic).to receive_message_chain(:where, :first).with(topic_name: row[:topic_name], assignment_id: session[:assignment_id]).with(no_args).and_return(nil) allow(ImportTopicsHelper).to receive(:define_attributes).with(row).and_return(attributes) allow(ImportTopicsHelper).to receive(:create_new_sign_up_topic).with(attributes, session).and_return(true) expect(ImportTopicsHelper).to receive(:create_new_sign_up_topic).with(attributes, session) SignUpTopic.import(row, session, nil) end end context 'when the topic is found' do it 'changes the max_chooser and topic_identifier of the existing topic' do row = {topic_name: 'name'} session = {assignment_id: 1} attributes = {topic_identifier: "identifier", topic_name: "name", max_choosers: "chooser", category: "category", description: "description", link: "link"} allow(SignUpTopic).to receive_message_chain(:where, :first).with(topic_name: row[:topic_name], assignment_id: session[:assignment_id]).with(no_args).and_return(topic) allow(topic).to receive(:save).with(no_args).and_return(true) expect(topic).to receive(:save).with(no_args) SignUpTopic.import(row, session, nil) end end context 'when the record has more than 4 items' do let(:row) do {name: 'no one', fullname: 'no one', email: 'name@email.com', role:'user_role_name', parent: 'user_parent_name'} end let(:attributes) do {role_id: 1, name: 'no one', fullname: 'no one', email: '', email_on_submission: 'name@email.com', email_on_review: 'name@email.com', email_on_review_of_review: 'name@email.com'} end before(:each) do allow(ImportFileHelper).to receive(:define_attributes).with(row).and_return(attributes) allow(ImportFileHelper).to receive(:create_new_user).with(attributes, {}).and_return(double('User', id: 1)) end end end end
Fix bugs in the tests for SignUpTopic
#spec/models/sign_up_topic_spec.rb describe SignUpTopic do let(:topic) { build(:topic) } describe '.import' do context 'when record is empty' do it 'raises an ArgumentError' do expect { SignUpTopic.import({}, nil, nil) }.to raise_error(ArgumentError, 'The CSV File expects the format: Topic identifier, Topic name, Max choosers, Topic Category (optional), Topic Description (Optional), Topic Link (optional).') end end context 'when record is not empty' do row = {topic_identifier: 'identifier', topic_name: 'name', max_choosers: 'chooser', category: 'category', description: 'description', link: 'link'} session = {assignment_id: 1} attributes = {topic_identifier: 'identifier', topic_name: 'name', max_choosers: 'chooser', category: 'category', description: 'description', link: 'link'} context 'when the topic is not found' do it 'creates a new sign up topic' do allow(SignUpTopic).to receive_message_chain(:where, :first).with(topic_name: row[:topic_name], assignment_id: session[:assignment_id]).with(no_args).and_return(nil) allow(ImportTopicsHelper).to receive(:define_attributes).with(row).and_return(attributes) allow(ImportTopicsHelper).to receive(:create_new_sign_up_topic).with(attributes, session).and_return(true) expect(ImportTopicsHelper).to receive(:create_new_sign_up_topic).with(attributes, session) SignUpTopic.import(row, session, nil) end end context 'when the topic is found' do it 'changes the max_chooser and topic_identifier of the existing topic' do allow(SignUpTopic).to receive_message_chain(:where, :first).with(topic_name: row[:topic_name], assignment_id: session[:assignment_id]).with(no_args).and_return(topic) allow(topic).to receive(:save).with(no_args).and_return(true) expect(topic).to receive(:save).with(no_args) SignUpTopic.import(row, session, nil) end end end end
Task 3. Write a test case to handle import conflicts. ( Issue #329 and Issue #328)
- Thoroughly test the #import method in the team model. This includes the following test cases:
- Duplicates exist. Ignore the new team
- Duplicates exist. Replace the existing team with the new team
- Duplicates exist. Insert any new members to the existing team
- Duplicates exist. Rename the new team and import
- Duplicates exist. Rename the existing team and import.
What is done
//attach code