Is this a correct use of context? Also should the it “” description be more specific with the number of members or is this fine ? Or should I move this method in the team_activity class which has a unique team and a challenge. A team cannot take part in the same challenge(same name) twice.
require 'spec_helper'
describe Team do
it { should have_many(:members)}
it { should have_many(:memberships)}
it { should belong_to(:admin)}
it { should validate_presence_of(:name)}
it { should have_many(:team_activities)}
it { should have_many(:challenges)}
it { should respond_to(:slogan)}
it { should have_attached_file(:photo) }
it { should respond_to(:goal_for_challenge)}
describe "#goal_for_challenge" do
context "all members have joined before the challenge was choosen" do
it "should return the goal for the team by calculating number of members multiplied by the number of steps" do
team = create(:team)
challenge = create(:challenge)
team_goal = team.goal_for_challenge(challenge)
expect(team_goal).to eq 0
end
end
context "some members joined after the challenge started and some before" do
it "should return the goal for the team by calculating number of members multiplied by the number of steps" do
team = create(:team)
challenge = create(:challenge)
team_goal = team.goal_for_challenge(challenge)
expect(team_goal).to eq 0
end
end
context "all members joined after the challenge started" do
it "should return the goal for the team by calculating number of members multiplied by the number of steps" do
team = create(:team)
challenge = create(:challenge)
team_goal = team.goal_for_challenge(challenge)
expect(team_goal).to eq 0
end
end
end
end
Perhaps you can make the contexts more readable by structuring them this way:
context 'before challenge was chosen'
context 'before challenge started'
context 'after challenge started'
Your it´s all have the same text and you specify the how of the method. I would recommend against that. Just write it 'returns the team´s goal'. If you want to know how it´s done, look inside the class. If you change the implementation details some time into the future, you would have to adjust the text inside your test. I find that´s a tedious task to do.
There are other things you can refactor here but it looks as if you aren´t finished writing this test? Perhaps you can show the final test, then we can give more feedback?
Thanks :). Yeah I had just started to write it. This is what I came up with one of the scenarios. Didnt take in your suggestions yet.
context "all members have joined before the challenge was choosen" do
it "should return the goal for the team by calculating number of members multiplied by the number of steps" do
UserActivity.any_instance.stub(:start_time).and_return(Time.now)
team = create(:team)
challenge = create(:challenge)
users = create_list(:user,10)
team.members << users
team_activity = create(:team_activity,team:team, challenge:challenge)
team_goal = team_activity.goal
expect(team_goal).to eq 0
end
end
def goal
total_goal = 0
team.members.each do |member|
user_activity = UserActivity.find_by(user:member, challenge:challenge)
start_date_of_user_activity = user_activity.start_time
difference_between_the_start_days = start_date_of_team_activity.day - start_date_of_user_activity.day
individual_goal = challenge.number_of_steps * difference_between_the_start_days
total_goal += individual_goal
end
return total_goal
end
It’s kind of hard to read code here, but three things that I’ve noticed right away:
You are creating all your objects on the database by using factory girl’s create method, you should try to use build_stubbed.
You are creating 10 users to test something, if you really need more than one different user, try to do it with 2. If you can just pretend to have a lot, and not actually create them, even better. The more the records/objects the slower your tests.
You could use timecop instead of stubbing the Time
I believe your code will not work immediately if you follow these ideas (especially number 1), but it will definitely push the code towards a better architecture.
You should also be on the lookout for code smells and possibilities of refactoring. Look at ruby science for a great resource on the topic.
This is the factories.rb I have written till now. It took me ages to come up with this version.
FactoryGirl.define do
factory :user do
user_name "ankur kothari"
sequence(:email) { |n| "person#{n}@example.com" }
password "12345678"
password_confirmation "12345678"
end
factory :challenge do
name "Walking Challenge"
number_of_steps 150000
number_of_days 30
visibility "Public"
end
factory :membership do
user
team
end
factory :user_activity do
start_time { (Time.now + 3.days) }
user
challenge
status "Participating"
factory :user_activity_with_daily_activity do
after(:create) do |user_activity, evaluator|
create_list(:daily_activity, user_activity.challenge.number_of_days,user_activity: user_activity,start_time:Time.now)
end
end
end
factory :daily_activity do
sequence(:steps_covered,0) {|n| n*1000 }
end
factory :team do
name "Getfitgo"
association :admin, factory: :user
factory :team_with_members do
after(:create) do |team, evaluator|
create_list(:membership, 5,team: team)
end
end
end
factory :team_activity do
start_time { (Time.now + 1.days) }
team
challenge
after(:create) do |team_activity|
team_activity.team.members.each do |member|
create(:user_activity_with_daily_activity, user:member,challenge:team_activity.challenge,start_time:team_activity.start_time)
end
end
end
end
The only problem you might have is with the after(:create) which I’m not sure will be ran with build_stubbed. I believe, however, that there’s an after(:build).
