Skip to content

Added basic test cases for resume model #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added basic test cases for resume model #52

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 79 additions & 1 deletion spec/models/resume_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,83 @@
require 'spec_helper'

describe Resume, "when created with blank parameters" do
before(:each) do
@resume = Resume.create
end

it "should have a file" do
@resume.should_not be_valid
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One guideline for writing good tests is that you shouldn't be testing the same thing in multiple test cases. In this case, you can have a separate test for testing "should not be valid". In fact, you might even just leave out the check for "should_not be_valid" because the other tests imply that the resume isn't valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm kind of confused here. Is the issue that I'm testing should_not be_valid in multiple test cases?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly. Ideally, you should be able to tell what a bug is from which tests are failing. The way they're currently written, if a resume isn't valid when created with no parameters, then every one of these tests will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if there's a test that basically says it "should not be valid", what am I supposed to write regarding test cases for the various validations in the model (presence, range, etc.)? I was modeling the test after some of the other model specs here, but they also repeatedly test should_not be_valid.

Sorry about asking so many questions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of our tests are honestly not written very well. You should be able to just remove all the should_not be_valid statements, since the @resume.errors checks already imply not valid.

@resume.errors[:file].should include("can't be blank")
end

it "should have an overall GPA" do
@resume.should_not be_valid
@resume.errors[:overall_gpa].should include("can't be blank")
end

it "should have a graduation year and semester" do
@resume.should_not be_valid
@resume.errors[:graduation_semester].should include("can't be blank")
@resume.errors[:graduation_year].should include("can't be blank")
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rspec protip: You can use the subject method to define a subject for a describe block and write one-line it statements. See http://betterspecs.org/#subject.

E.g.

describe Resume, "when created with blank parameters" do
  subject { Resume.create }
  it { should validate_presence_of(:file) }
  ...
end

Not sure if validates_presence_of comes with Rspec, but it's available from the shoulda gem.

end

describe Resume do
pending "add some examples to (or delete) #{__FILE__}"
before(:each) do
@good_params = {:overall_gpa => 4.0,
:major_gpa => 4.0,
:resume_text => "The resume of Genius McGenius",
:graduation_year => 2016,
:graduation_semester => "Spring",
:file => "private/resumes/geniusmcgenius.pdf",
:person => mock_model(Person)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Hashes where the closing brace is on its own line, I personally like the style where the first item is on its own line and everything is indented by just one indent level:

variable = {
  :item1 => 'foo',
  :item2 => 'bar',
  :item3 => 'baz',
}

}
end

it "should accept valid parameters" do
resume = Resume.create(@good_params)
resume.should be_valid
end

it "should reject invalid GPAs" do
resume = Resume.create(@good_params)
resume.update_attribute(:overall_gpa, 5.0)
resume.should_not be_valid
resume.errors[:overall_gpa].
should include("must be less than or equal to 4.0")

resume = Resume.create(@good_params)
resume.update_attribute(:major_gpa, 5.0)
resume.should_not be_valid
resume.errors[:major_gpa].
should include("must be less than or equal to 4.0")

resume = Resume.create(@good_params)
resume.update_attribute(:overall_gpa, -1.0)
resume.should_not be_valid
resume.errors[:overall_gpa].
should include("must be greater than or equal to 0.0")

resume = Resume.create(@good_params)
resume.update_attribute(:major_gpa, -1.0)
resume.should_not be_valid
resume.errors[:major_gpa].
should include("must be greater than or equal to 0.0")
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another testing guideline is to make sure each test is just testing one thing. This test case can be broken up into four separate test cases, and you can still group them under a single context block. This will also allow you to factor out the initial Resume.create, since each of these use the same parameters.


it "should reject non numeric graduation years" do
resume = Resume.create(@good_params)
resume.update_attribute(:graduation_year, "Nineteen Eighty-Four")
resume.should_not be_valid
resume.errors[:graduation_year].
should include("is not a number")
end

it "should reject non integer graduation years" do
resume = Resume.create(@good_params)
resume.update_attribute(:graduation_year, 2012 + Math::PI)
resume.should_not be_valid
resume.errors[:graduation_year].
should include("is not a number")
end
end