everydayrails / rspec_rails_4 Goto Github PK
View Code? Open in Web Editor NEWEveryday Rails Testing with RSpec (Rails 4.0 Edition)
License: MIT License
Everyday Rails Testing with RSpec (Rails 4.0 Edition)
License: MIT License
The contact.rb
example on page 43 (bottom) is a bit confusing.
According to the book we can "just add" the validation in the example to the contact model, and everything will work. However this was not the case for me. When I added the validation rule the filter last name by letter
test failed. This test isn't mentioned anywhere in the chapter, and thus hasn't been updated to use FactoryGirl unlike all the other tests. I was a bit confused by this, but in the end I converted it to FactoryGirl myself. Perhaps it would be a good idea to have the converted version in the book with the other tests to avoid any confusion.
In relation to this issue I looked in the source code on branch 04_factories and discovered the following FactoryGirl version of the test:
describe "filter last name by letter" do
before :each do
@smith = create(:contact,
lastname: 'Smith', email: '[email protected]')
@jones = create(:contact,
lastname: 'Jones', email: '[email protected]')
@johnson = create(:contact,
lastname: 'Johnson', email: '[email protected]')
end
....
end
This was different from my own test in that I didn't manually specify email adresses for each contact. Is this necessary? In my optics this test doesn't care about email addresses and will be happy with those randomly generated by Faker & FactoryGirl.
Book version: 2013-10-29 (PDF)
Hi @ruralocity, I found one issue when translating Chapter 7.
P83, exercise 2:
expect(@message.reload.is_inappropriate?).to be_true
should be
expect(@contact.reload.hidden?).to be_true
On page 42, line 1:
I could keep using sequences and my specs would still pass. But does give us a bit more realistic..
One or more words are missing somewhere around "But".
Book version: 2013-10-29 (PDF)
In new version of book, chapter 5, the spec example names in page 49 and the names given with implementation in following pages are different...
There's a slight code style inconsistency between the GET #edit
and other specs on page 69. Specifically, the #edit spec uses an intermediary variable to store the created contact while the #create spec and others don't.
Book version: 2013-10-29 (PDF)
Page 59, second-last line:
...hiding contacts from view without deleted them (I'll..
-->
...hiding contacts from view without deleting them (I'll..
Book version: 2013-10-29 (PDF)
There is a mistake in the following code. Where you said "message", maybe you wanted to say "contact".
it "redirects to the home page upon save" do
post :create, message: FactoryGirl.attributes_for(:contact) expect(response).to redirect_to root_url
end
In "Automation with Guard and Spork" section, there is a warning box, says "the version of Spork included in the guard-spork
gem published on Rubygems isn't quite ready for Ruby 2.0.0", and gives a link to source code, but the link is broken, and maybe should mention which files are modified.
The contacts.rb
and phones.rb
examples on page 41 and 42 both have the require 'faker'
statement. I accidentally left it out in my Phone factory, but it seems everything works fine without it. I have no idea why, but I have now also removed it from the Contact factory and everything seems to work fine - less is more, right :-)
Book version: 2013-10-29 (PDF)
The following issues are regarding the code example on the top of page 26 as well as the first line after this example:
This spec uses RSpec's include matcher to determine if the array returned by
Contact.by_-letter("J")
-and it passes!
Three issues here:
Contact.by..
in the text is incorrectly hyphenated which makes it look like the hyphen is a part of the method name. I've seen this in another LeanPub book recently - it may be more than a local issue?Contact.by_-letter("J")
. and "and it passes!".Contact.by_letter("K")
, Contact.by_letter("X")
and so on. Clearly these fit your description of returning no results.by_letter
method returns only the correct results. (and by the way shouldn't the test name thus be something like 'doesn't return non-matching results'?).Book version: 2013-10-29 (PDF)
The PUT #update spec on page 50 has a minor inconsistency in naming:
describe 'PUT #update' do
context "with valid attributes" do
it "updates the contact in the database"
it "redirects to the contact"
end
context "with invalid attributes" do
it "does not update the contact"
it "re-renders the #edit template"
end
end
In the first spec it says updates the contact in the database while the second spec just says does not update the contact but fails to specify where the update should not occur (in the database). This is a very minor thing, really.
Book version: 2013-10-29 (PDF)
In the code example on page 50, the #update test is referred to as PUT #update, but it should really be PATCH #update as mentioned on page 57.
Book version: 2013-10-29 (PDF)
"So it Try Git." should probably be "So is Try Git."
In the test populates an array of contacts starting with the letter on page 53, the Jones contact is assigned to an unused jones
variable:
it "populates an array of contacts starting with the letter" do
smith = create(:contact, lastname: 'Smith')
jones = create(:contact, lastname: 'Jones')
get :index, letter: 'S'
expect(assigns(:contacts)).to match_array([smith])
end
The assignment is unnecessary.
Furthermore, I suggest reversing the order of the creates. As it is now the test would pass if the production code always returns a one-element array with the first contact in the database (a plausible mistake), because the test first creates smith
and then tests that [smith]
is returned. Instead, by first creating jones
and checking that [smith]
is returned, you require the production code to at least do something intelligent because it needs to pass over jones
and only return smith
.
To be honest an even better test would create four contacts and check that the two in the middle are returned. That would cover more edge cases than the current one and be more representative of the desired behavior.
Book version: 2013-10-29 (PDF)
In the first test in phone_spec.rb
on page 40 the created home phone is stored in the variable home_phone
, but the variable is not used in the test.
Book version: 2013-10-29 (PDF)
On page 39, line 5 we have the following:
So far, if we wanted to specify a non-home phone in a spec we've done it like this:[code example follows]
There are two issues here:
To sum up: rephrase the sentence to focus on the fact that IF we wanted to use different phone number types, THEN it would look like the example. Also, use an example that actually illustrates the point by creating two different phone number types.
Book version: 2013-10-29 (PDF)
In the contact_spec.rb
code example on page 38 the has a valid factory
test suddenly uses create
instead of build
compared to the not-so-lean test on page 36. I don't believe create
is necessary here.
Book version: 2013-10-29 (PDF)
In the following code, where you said "Message", maybe you wanted to say "Contact".
expect(Message.to_csv).to match /Aaron Sumner,[email protected]/
In the chapter 5, there are other places where I guess you exchanged "contact" for "message", but these other mistakes do not compromise the understanding of the text.
This is a great book, thank you!
The Gemfile contains the following outdated gems:
The lines just below the Gemfile state that:
These are the current versions of each gem as of this writing.
which is not entirely true since the book was updated a few days ago, and some of these gems were updated much longer ago.
I've made it to chapter 4 with the newer versions without any problems so far.
Book version: 2013-10-29 (PDF)
Is there a difference between a template and a view? If not then only one of them should be used for consistency, if yes, then maybe add a note about it in the book to avoid confusion. It seems like the two are used interchangeably, e.g on page 49 in contacts_controller_spec.rb
the following test says 'renders the :index view':
context 'with params[:letter]' do
it "populates an array of contacts starting with the letter"
it "renders the :index view"
end
but a couple of lines later it says 'renders the :show template':
describe 'GET #show' do
it "assigns the requested contact to @contact"
it "renders the :show template"
end
Two issues with the phone_spec.rb
example on page 22:
phone_type:
symbols not coloered blueish as the other symbols (e.g. firstname:
, lastname:
)?home_phone
? It's not used in the test.Book version: 2013-10-29 (PDF)
On page 42, section Advanced associations, second-last line:
..contact factory uses the after callback make sure that a new contact.."
A to is missing between callback and make.
Book version: 2013-10-29 (PDF)
The spec on page 57 is completely different from the one originally outlined on page 50. For example the one on page 50 has a test updates the contact in the database
but the one on page 57 doesn't. Instead it has a changes @contact's attributes
that the one on page 57 doesn't have. Is this intentional?
Book version: 2013-10-29 (PDF)
Page 31, section "How DRY is too DRY"?, third-last line:
Even better, when we get into testing users with specific roles in chapter 6, might be variables like @admin_user and @guest_user.
There's a word or two missing in this sentence. Did you mean "..there might be variables like.."?
Book version: 2013-10-29 (PDF)
Page 57, contacts_controller_spec.rb
:
it located the requested @contact
-->
it locates the requested @contact
Book version: 2013-10-29 (PDF)
On page 29 in the code example contact_spec.rb
the spec constructed through the chapter is shown in its full. But, where did the "is invalid without email address" spec body come from? It's block is included in the skeleton created in the beginning of the chapter (page 17), but it isn't mentioned anywhere until here where the full body appears. All the other specs are mentioned and the reader is guided through writing them in the chapter. At least there should be a note about leaving this spec to the reader as an exercise if that's the intention.
Book version: 2013-10-29 (PDF)
The spec on page 59 is different from the one originally outlined on page 50. For example the one on page 50 has a test redirects to users#index
but the one on page 57 doesn't. Instead it has a redirects to contacts#index
test. Is this intentional?
Book version: 2013-10-29 (PDF)
This is more a question than an actual issue (and related to one of my other issues with creating contacts, see #38).
Does the deletes the contact
test on page 59 really test that the contact is deleted?
it "deletes the contact" do
expect{
delete :destroy, id: @contact
}.to change(Contact,:count).by(-1)
end
In my opinion this test does nothing but test that some contact is deleted (and thus should really be named deletes some contact
). The deleted contact could be any contact, but there's nothing that ensures it's the one with the correct id. Should we not test this? Isn't it a plausible bug to delete a wrong contact?. I know we can't discover ALL bugs with tests, but in this case it seems obvious that the test doesn't test what it says it does and can be improved.
Book version: 2013-10-29 (PDF)
This is more a question than an actual issue.
Regarding the spec on page 53, what about the requirement that contacts should be returned in sorted (ascending) order according to the last name? There's no spec to check this in the controller spec. Should there be? The contact model spec covers this requirement, but should it also be tested in the controller? Why/why not?
In general it seems the book doesn't address this issue at all - e.g. which features should we test at which layers? I'm only at chapter 6 so it may do so later, but if not, consider adding a note about this.
Book version: 2013-10-29 (PDF)
The Gemfile in the 02_setup branch of the code uses version 2.33.0 of selenium-webdriver
, but the corresponding code in the book (page 9) uses version 2.35.1.
Both versions are outdated (see my earlier issue report), but they should at least be consistent.
Book version: 2013-10-29 (PDF)
It seems the book has no consistency in its use of ' vs " for strings. Sometimes ' is used in describe blocks, sometimes " is used, sometimes ' is used in context blocks and sometimes not. I feel it would be more professional and less confusing to chose a convention and stick to it.
Book version: 2013-10-29 (PDF)
The _redirects to contacts#show" test on page 55 has a subtle bug:
it "redirects to contacts#show" do
post :create, contact: attributes_for(:contact, phones_attributes: @phones)
expect(response).to redirect_to contact_path(assigns[:contact])
end
Look at what RSpec will output if ContactsController#create doesn't assign @contact
:
2) ContactsController POST #create with valid attributes redirects to contacts#show
Failure/Error: expect(response).to redirect_to contact_path(nil)
Expected response to be a redirect to <http://test.host/contacts/> but was a redirect to <http://test.host/contacts/1>.
Expected "http://test.host/contacts/" to be === "http://test.host/contacts/1".
# ./spec/controllers/contacts_controller_spec.rb:101:in `block (4 levels) in <top (required)>'
This tells us nothing about what the failure is (well, it does as it says contact_path(nil)
which hints to the problem, but the message is still confusing). It is misinforming because it says we expect to be redirected to "http://test.host/contacts/" but we were redirected to "http://test.host/contacts/1" which is the correct url.
The problem is subtle. By using contact_path(assigns[:contact])
the test implicitly expects assigns[:contact]
to be not nil. The problem is the implicitness; if assigns[:contact]
returns nil contact_path(nil)
returns /contacts/ which is equivalent to contacts_path
. I don't know if this is a bug or a feature in Rails - I don't see why contact_path(nil)
should ever be a valid path since you can always use contacts_path
instead? Anyway, this is why the test fails with a wrong message.
To solve this one should at least make the assumption explicit by explicitly expecting assigns[:contact]
to be not nil:
it "redirects to contacts#show" do
post :create, contact: attributes_for(:contact, phones_attributes: @phones)
assigned_contact = assigns(:contact)
>> expect(assigned_contact).to_not be_nil <<
expect(response).to redirect_to contact_path(assigned_contact)
end
This test will fail in the correct way when @contact
isn't assigned and also expresses the intent better.
A related question is whether expecting @contact
to be not nil is good enough. Do we not want to test that it's assigned the correct contact and not just any contact? For example with a test like this:
it "assigns the new contact to @contact" do
contact_attrs = attributes_for(:contact)
post :create, contact: contact_attrs
assigned_contact = assigns(:contact)
expect(assigned_contact).to_not be_nil
expect(assigned_contact.attributes).to include(contact_attrs.stringify_keys)
end
Note that this doesn't test for correctness of phones - I couldn't figure out how to do that.
Book version: 2013-10-29 (PDF)
The spec on page 70 has no space between the HTTP method and the controller action, e.g. GET#index
instead of GET #index
(which the previous part of the book has used consistently).
Book version: 2013-10-29 (PDF)
In the pdf version of the book, on page 33, there's a paragraph starting with:
Edit the parameters passed to the factory within the expectation.
But we're not actually using factories yet, are we?
Hi @ruralocity, I encountered some issues in chapter 5 when translating.
:invalid_message
should be :invalid_contact
.link_to 'Export', messages_path(format: :csv)
, should be link_to 'Export', contacts_path(format: :csv)
.defindex
@contacts = Contact.all
respond_to do |format| format.html # index.html.erb format.csv do
send_data Contact.to_csv(@contacts),
type: 'text/csv; charset=iso-8859-1; header=present',
:disposition => 'attachment; filename=contacts.csv'
end
end
end
I think wen should sticky with the new Hash syntax here.
it "returnscommaseparatedvalues" do
create(:contact,
firstname: 'Aaron',
lastname: 'Sumner',
email: '[email protected]')
expect(Message.to_csv).to match /Aaron Sumner,[email protected]/
end
Here the Message
should be Contact
.
it "returnsJSON-formattedcontent" do
message = create(:message)
get :index, format: :json
expect(response.body).to have_content message.to_json
end
Since we do not manage messages in this new version, I think this example should use contact.
In the Introduction is this bullet:
• RSpec 2.14: I began writing this book while version 2.8 was current, so anything you see here should apply to versions of RSpec at least that far back.
Since 2.8 is > 2.14, I assume this is meant to say 1.8?
As the book has been updated to use RSpec's newer expect()
syntax, would it be worth updating the Mocks and Stubs section to use the newer expectation syntax too?
e.g.
Contact.stub(:persisted?).and_return(true)
could now be
allow(Contact).to receive(:persisted?).and_return(true)
Of course both are fine, but it seems like RSpec is moving in the direction of the latter.
This is more a question than an actual issue.
In the spec on page 55, does the saves the new contact in the database test really test that the new contact is saved?:
it "saves the new contact in the database" do
expect{
post :create, contact: attributes_for(:contact, phones_attributes: @phones)
}.to change(Contact, :count).by(1)
end
In my opinion this test does nothing but test that some contact is saved (and thus should really be named saves some new contact in the database). The saved contact could be any valid contact, but there's nothing that checks the saved contact is actually the one submitted. Should we not test this?
Book version: 2013-10-29 (PDF)
In the pdf version of the book, on page 43, there's a sentence starting with:
But does give us a bit more realistic data...
Shouldn't it be as such?:
But this does give us a bit more realistic data...
This code example on page 67 should just be the previous spec code (from chapter 5) surrounded by a describe block that logs in as admin. However, the spec code illustrated inside the describe block is vastly different from the one in chapter 5 and thus quite confusing. Just to mention a few differences:
@contact
variable in all tests instead of the local variables in chapter 5populates an array of contacts starting with the letter
but the equivalent test in this code seems to be populates an array of contacts
with a different body)Book version: 2013-10-29 (PDF)
Looks like selenium-webdriver versions before 2.35.1 had a loose dependency on rubyzip, and when the latter was update to 1.0.0 caused a LoadError in ActiveSupport. Fix this by either requiring selenium-webdriver ~> 2.35.1 or rubyzip < 1.0.0 in your Gemfile.
I will update the sample source and book this week. Thanks to reader Geoffroy for pointing this out.
Just a small issue, but there's this in chapter 3. Model specs
"We’ll expand this outline in a few minutes, but this gives us quite a bit for starters. It’s a simple spec for an admittedly simple model, but points to our first three best practices:"
Then you list four best practices.
If i happen to find anything else i'll let you know.
Thanks for the good work so far!
我又新写了一章,通过为演示程序添加新功能,延时了 TDD 流程
应该是
我又新写了一章,通过为演示程序添加新功能,演示
了 TDD 流程
Some code examples use nicely aligned method arguments (e.g. page 20) while others have no alignment at all (e.g. page. 22, 24, 25). The aligned versions are much easier to read.
Consider aligning all examples in the book to both be consistent and clear.
Book version: 2013-10-29 (PDF)
In line 3 of the code in this section, expect(attribute).to
should instead read expect(actual).to
, since you're testing what actually happened instead of an expected attribute (which isn't even passed in in this example).
It becomes clearer if you look at RSpec Wiki's sample code that you link to at the end of the section: https://github.com/dchelimsky/rspec/wiki/Custom-Matchers They use expected
instead of attribute
, which makes the distinction a little clearer.
Thanks for a really great book!
Should this be Rails 4 in the updated version?
"In Everyday Rails Testing with RSpec I’ll walk you through taking a basic Rails 3.2"
On page 62 the to_csv
method takes an argument:
format.csv do
send_data Contact.to_csv(@contacts),
type: 'text/csv; charset=iso-8859-1; header=present',
disposition: 'attachment; filename=contacts.csv'
end
but on page 63 it doesn't:
expect(Contact.to_csv).to match /Aaron Sumner,[email protected]/
How does the method know which contact to convert to csv here? If it just converts ALL contacts without arguments, then why is an argument given on page 62? Maybe add a note about how this works to clarify.
Book version: 2013-10-29 (PDF)
The spec on page 59 (and the corresponding source code) is missing a # (e.g. compared to the original spec on page 50):
describe 'DELETE destroy' do
-->
describe 'DELETE #destroy' do
Book version: 2013-10-29 (PDF)
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.