CSC/ECE 517 Spring 2023 E2310
About Expertiza
Expertiza is an assignment/project management portal that can be used by both instructors and students for collaborative learning and feedback. It is an open-source project based on Ruby on Rails framework. It allows the instructors not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams to work on various projects and assignments. Expertiza also lets students peer-review other students' submissions, enabling them to work together to improve others' learning experiences.
Team
Mentor
- Jialin Cui
Team Members
- Aman Waoo
- Girish Wangikar
- Pranavi Sharma Sanganabhatla
Description and Testing
Our project deals with refactoring the answer.rb file. The code is responsible for doing the following tasks:
- Get all answers for each question and send them to summarization WS.
- Get average scores and a summary for each question in a review by a reviewer.
Files Involved
- answer.rb
- answer_spec
Running Tests
To successfully run answer_spec on the local machine, please run the below rspec command.
rspec spec/helpers/answer_spec.rb
Project Description
The objective of the project is to refactor the following Ruby on Rails model classes in the existing project:- SignUpTeam, SignUpTopic and SignUpSheet. SignUpTeam model class represents a signed-up team for a particular sign-up topic in a specific assignment. SignUpTopic model class represents a topic that can be signed up for in a course assignment. SignUpSheet class stores the logic for creating a signed_up_team for a topic for a particular assignment. This class is being used as a module having all class methods.
The task is to refactor the existing code present in these model files and implement them in a new repository. Since the project is implemented from scratch, there are various aspects of the project that have to be implemented and therefore unit test cases are implemented to test the implementation.
Issue Description
- The SignedUpTeam class has methods that handle multiple responsibilities, including finding participants and teams, releasing topics, and removing signed up teams. This violates the single responsibility principle and makes the class harder to understand and maintain.
- Some of the queries in the class use complex SQL statements and multiple joins, which can make them hard to read and understand. These complex SQL queries in the code could be simplified and made more readable by using ActiveRecord's query interface instead.
- The same code is repeated in multiple methods, such as the code to find signed up teams for a given topic. This violates the Don't Repeat Yourself (DRY) principle and can make the code harder to maintain.
Test Plan
After going through all the methods in the class, we figured out there were some methods that were never being called. So, we removed all the redundant methods that are not being called. Now we had only 9 methods in the class that we need to test. Nine methods in summary_helper class that needed to be tested were:
- get_sentences
To test this method we mocked an answer object and checked if the number of sentences is equal to the number of comments provided in the answer. We identified two test cases for this method: 1. When the answer is nil 2. When the comment is two sentences
Code Snippet:
describe '#get_sentences' do context 'when the answer is nil' do it 'returns a nil object' do expect(@summary.get_sentences(nil)).to eq(nil) end end context 'when the comment is two sentences' do it 'returns an array of two sentences' do sentences = @summary.get_sentences(answer) expect(sentences.length).to be(2) end end end
- get_max_score_for_question
To test this method we mocked two different types of questions which are as follows: 1. let(:questionOne){Question.new(type:'Checkbox')} 2. let(:questionTwo) { build(:question, questionnaire: questionnaire1, weight: 1, id: 1) } Now based on the different question types we tested two scenarios for this method: 1. When the question type is Checkbox 2. When question type is not Checkbox
Code Snippet:
describe 'get_max_score_for_question' do context 'When question type is Checkbox' do let(:questionOne){Question.new(type:'Checkbox')} it 'returns 1' do max_score = @summary.get_max_score_for_question(questionOne) expect(max_score).to be(1) end end context 'When question type is not Checkbox' do let(:questionnaire1) { build(:questionnaire, id: 2) } let(:questionTwo) { build(:question, questionnaire: questionnaire1, weight: 1, id: 1) } it 'return the max score for the provided question' do allow(Questionnaire).to receive(:where).with(id:2).and_return(questionnaire1) allow(questionnaire1).to receive(:first).and_return(questionnaire1) expect(@summary.get_max_score_for_question(questionTwo)).to eql(5) end end end
- summarize_sentence
To test this method we mocked a comments array with the following values ["Hello this is first comment", "This is second comment"]. This method is making a web service call at the following Web Service: 'http://peerlogic.csc.ncsu.edu/sum/v1.0/summary/8/lsa' We are using expect to compare the expected result with the actual result from the web service call. Disclaimer: This web service call results in a bad gateway error while running the test case. We have already informed our mentor and the professor regarding the same.
Code Snippet:
describe '#summarize_sentence' do context 'successful webservice call' do comments = ["Hello this is first comment", "This is second comment"] summary_ws_url = WEBSERVICE_CONFIG['summary_webservice_url'] it 'return success' do expect(@summary.summarize_sentences(comments,summary_ws_url)).not_to eql(nil) end end end
- break_up_comments_to_sentences
This methods question answer array and breaks them into comments. In our testing we provided the question_answer array of length 2 and see if the number of comments is equal to 2.
Code Snippet:
describe '#break_up_comments_to_sentences' do context 'when the question_answers is not nil' do it 'add the comment to an array to be converted as a json request' do comments = @summary.break_up_comments_to_sentences([answer]) expect(comments.length).to be(2) end end context 'when the question_answers is nil' do it 'returns an empty array' do comments = @summary.break_up_comments_to_sentences([]) expect(comments.length).to be(0) end end end
- calculate_avg_score_by_criterion
This test is to see when question answer was given, the output of method is correctly calculated as percentage of question score. Code Snippet:
describe '#calculate_avg_score_by_criterion' do context 'when question_answers are available' do it 'calculate percentage question_score & no float' do expect(@summary.calculate_avg_score_by_criterion([answer,answer1], 3)).to be_within(0).of(50) end end
This test is to see when question answer was nil, the output of method is correctly calculated as 0.
Code Snippet:
context 'when question_answers are not available' do it 'gives question scores 0.0' do expect(@summary.calculate_avg_score_by_criterion([], 3)).to eq(0.0) end end
This test is to see when q_max_score = 0, the output of method is correctly calculated.
Code Snippet:
context 'when q_max_score = 0' do it 'gives pure question_score' do expect(@summary.calculate_avg_score_by_criterion([answer,answer1], 0)).to eq(3.0) end end end
- calculate_round_score
The test is to see when the criteria input is nil, the method gives right output of nil, which is 0 in this case. Code Snippet:
describe '#calculate_round_score' do context 'when criteria not available' do it 'returns 0.0 since round_score = 0.0' do expect(@summary.calculate_round_score(avg_scores_by_criterion, nil)).to eq(0.to_f) end end
The test is to see when the criteria input is not nil, the method gives the right output, which is to get a two-digit round score. The criteria input was defined as question in mock section.
Code Snippet:
context 'when criteria not nil' do it 'get 2 round_score ' do expect(@summary.calculate_round_score(avg_scores_by_criterion, question)).to be_within(0.01).of(2.345) end end end
- calculate_avg_score_by_round
This test is to see the method round an average number up to Two-digit. The input of avg_scores_by_criterion was given as 2.345, and we tested here that it gives two-digit round number as 2.35. Code Snippet:
describe '#calculate_avg_score_by_round'do context 'when avg_scores_by_criterion available' do it 'gives 2 round value' do expect(@summary.calculate_avg_score_by_round(avg_scores_by_criterion, question)).to eq(2.35) end end end
Test Execution
We divided the work among the teammates and started tackling the problems. We stubbed the data using factory and mocked the method calls which were being done internally to get the desired output from the methods that were calling other methods internally.
Test Coverage
The summary helper spec file is newly created. This project increased testing coverage by 72.86% according to simplecov test.
Test Plan
We have implemented test cases using RSpec for different validation checks for both sign_up_team.rb and sign_up_topic.rb. The other test cases include 'create' tests that validate whether the utility methods can create the records in the table as expected. Moreover, 'update' test cases are included to validate whether the modifications are updated in the table. Finally, we perform 'destroy' tests to validate whether the record is deleted from the table. We also added unit tests that validate other instance methods, such as format_for_display, in both SignUpTeam and SignUpTopic model classes. We created helper stubs for the new methods yet to be implemented to perform the testing process smoothly. We have attached the unit test assessment video as shown here
Future Plan
The future scope of this project is to implement waitlist.rb model class that handles the functions related to adding/removing sign up teams to the waitlist for each topic. There are other instance methods in both sign_up_team.rb and sign_up_topic.rb that requires implementation. But, since they are dependent on other model classes, we have added them to our TODO list to implement them in the future.
Conclusion
While we tried to test all the methods in the summary_helper.rb class, we faced some blockers. All the blockers are described below:-
- summarize_sentences method
summary_webservice_url: 'http://peerlogic.csc.ncsu.edu/sum/v1.0/summary/8/lsa' This URL is being used in the summary_helper.rb file and we were not able to mock it. It's giving us bad gateway error while we try to hit that url.
- summarize_reviews_by_reviewee method
In this method we found out in the loop, questions[round] is not the correct way of passing the individual question since questions is an array and use 0 based indexing. Whereas in the above logic it is being treated as a hash.
- summarize_reviews_by_reviewee_question method
This method uses the instance variables from a different method(summarize_reviews_by_reviewee) which are not being passed as arguments to this method. Technically this type of initialization is bad coding practice and hence needs refratoring.
We tested all other remaining methods in the summary_helper.rb class. All the unit tests passed and the methods were working as expected. We tried to cover corner cases but there is room for some improvement.
Test Plan
We have implemented test cases using RSpec for different validation checks for both sign_up_team.rb and sign_up_topic.rb. The other test cases include 'create' tests that validate whether the utility methods can create the records in the table as expected. Moreover, 'update' test cases are included to validate whether the modifications are updated in the table. Finally, we perform 'destroy' tests to validate whether the record is deleted from the table. We also added unit tests that validate other instance methods, such as format_for_display, in both SignUpTeam and SignUpTopic model classes. We created helper stubs for the new methods yet to be implemented to perform the testing process smoothly. We have attached the unit test assessment video as shown here File:Test coverage.mp4
Future Plan
The future scope of this project is to implement waitlist.rb model class that handles the functions related to adding/removing sign up teams to the waitlist for each topic. There are other instance methods in both sign_up_team.rb and sign_up_topic.rb that requires implementation. But, since they are dependent on other model classes, we have added them to our TODO list to implement them in the future.