I was basically about to sag the same thing.i want to add though that I’ve seen this “should” approach greatly help people manage the mental switch from the test_functionname pattern, but yeah, once you feel okay take off the training wheels
This all just feels like a ton of unnecessary noise to me. I actually do test “should return error code 1 for unknown error” right here: https://github.com/carlmjohnson/exitcode/blob/master/code_test.go#L21 The test is called TestGet_default. I think if it were somehow confusing “why is it checking for 1? what does 1 mean here?” it might be worth having a comment or a longer test name, but in most cases what the test is testing for should be obvious and if it’s not (like the discount example), then that means the API in the code probably isn’t as clear as it should be.
Agreed. I’m not a fan of verbose test names like this, especially when it’s done using one of those cutesy DSLs, like it().should().be().equalToOrGreater().than(42). Just call it TestRange or TestMin or whatever.
Nit: the test above is actually called TestGet/default. See go test -v ./... output:
Good point! I should’ve mentioned that I’ve mostly used test frameworks which require a certain naming convention to detect tests, such as Pytest’s default test_ prefix, which can be overridden but (AFAIK) not avoided completely.
Heh. I hate this. And I’m the person who either invented this or popularised it. (Not sure which I did, but it was definitely one of the two. And I’m sorry.)
I find “should update count when …” is less good than “updates count when…”, especially because the popular frameworks these days use describe/it syntax (definitely my fault, and I’m sorry) and “describe AThing, it updates the count when …” reads better without should everywhere.
My test names answer the question “what does the software do when it’s working as described?” And “should” doesn’t answer that question: it doesn’t should do something, it does something.
I should (ha!) elaborate. Basically, “should” is language that has overstayed its welcome.
should was originally used in place of test because we were trying to get away from “test-centric language” where it caused confusion. So we made test runners that ran should_* instead of test_*. should is a better word than test when people are getting hung up on the word “test.”
But now we have better tools and language to write these things, and the word “test” doesn’t exist in many of the frameworks. Where there is no test there ought not be should. Where this is test, should is probably better. But we have better ways of writing this stuff in most languages now.
(I’m also less convinced that the word test is bad than I was in the early 2000s.)
Still, it was (at least for me) a groundbreaking new way of thinking about tests and how to do them. When I was doing Rails back in the day, RSpec felt like it really expanded my mind. Cheers to you for having advanced software engineering in a meaningful way!
Some read better than others. Other than that, I consider “should” a bit redundant. I prefer to be more firm in my assertions. “This method does this thing” instead of “this method should do this thing”.
test libraries that provide it syntax play well with this approach.
Mocha (Javascript):
describe('#myMethod()', function () {
it('returns 42', function () {
assert.equal(myMethod(), 42); });
});
RSpec (Ruby):
describe '#my_method' do
it 'returns 42' do
expect(my_method).to eq 42
end
end
I love little tricks like this where a particular bit of language can help you write readable code. Another one I really like is making sure commit messages complete the phrase “if merged, this commit will…”.
I think you misunderstood @dstaley’s comment. They’re not suggesting you write “If merged, this commit will add the ability to delete users” as the commit message, but “Add the ability to delete users”, using the imperative mood.
What you wrote is also good, I think, and it’s down to personal taste whether you want the imperative mood (”Add the ability to delete users”) or present tense (”Adds the ability to delete users”).
Yes please, great rule. I work with a group that enforces religious use of rebasing and useful commit messages in PRs. We don’t allow GitHub to do squash and merge. We follow your “if merged…” completion style, it just wasn’t defined in those terms. Well stated. So much nicer than what I’ve seen previously.
It removes redundancy, because the function name should already be in the call stack.
That implies that the only time you ever look at a test is when it fails. I spend a lot of time trying to navigate test organization to write new tests and review code. There’s no stack trace, so the tests just look jumbled together.
Test organization is one of the most frustrating topics, to me. It seems that every language/framework has a different approach to it, none are perfect, and it’s impossible to aggregate all the good ideas, due to technical difficulties. For example, there’s good aspects to the author’s idea, but I can’t use it in pytest because test names have to start with test_. I’ve largely broken down to using doc strings to explain tests, and I don’t love that that’s the best I can come up with.
Test organization is one of the most frustrating topics
Yes. I haven’t seen satisfactory solutions either. One thing which helps a bit are explicit coverage marks which sometimes help you to identify where tests for specific area live.
Jeff Forcier (the author of Invoke and the maintainer of Fabric and Paramiko) wrote a plugin for Pytest that lets you specify tests in this more literate fashion as well: https://github.com/bitprophet/pytest-relaxed
It’s not quite as expressive in this regard as rspec is, but it makes for nice readable test output when it’s the right model for your tests.
oh wow, i’ve wanted this so badly. installing immediately. between this and this, my comment has been a great use of time as i use both rust and python :)
Naturally, by default Rubocop will shout the opposite at you for your RSpec tests, because Rubocop wouldn’t be Rubocop if it didn’t have absolutely nonsensical defaults.
I find “should” in test names to be a noise word that can be easily eliminated.
Taking some of the examples from the article:
applies discount when total cost exceeds 100 dollars
creates record for valid input
returns error code 1 for unknown error
I was basically about to sag the same thing.i want to add though that I’ve seen this “should” approach greatly help people manage the mental switch from the test_functionname pattern, but yeah, once you feel okay take off the training wheels
This all just feels like a ton of unnecessary noise to me. I actually do test “should return error code 1 for unknown error” right here: https://github.com/carlmjohnson/exitcode/blob/master/code_test.go#L21 The test is called TestGet_default. I think if it were somehow confusing “why is it checking for 1? what does 1 mean here?” it might be worth having a comment or a longer test name, but in most cases what the test is testing for should be obvious and if it’s not (like the discount example), then that means the API in the code probably isn’t as clear as it should be.
Agreed. I’m not a fan of verbose test names like this, especially when it’s done using one of those cutesy DSLs, like
it().should().be().equalToOrGreater().than(42)
. Just call itTestRange
orTestMin
or whatever.Nit: the test above is actually called
TestGet/default
. Seego test -v ./...
output:Re: Nit, I couldn’t quite remember how t.Run gloms names together, thanks.
Good point! I should’ve mentioned that I’ve mostly used test frameworks which require a certain naming convention to detect tests, such as Pytest’s default
test_
prefix, which can be overridden but (AFAIK) not avoided completely.It definitely does depend on the testing framework you’re using!
As another reply pointed out, this works best when you’re using a DSL where you setup tests similar to this:
Heh. I hate this. And I’m the person who either invented this or popularised it. (Not sure which I did, but it was definitely one of the two. And I’m sorry.)
I find “should update count when …” is less good than “updates count when…”, especially because the popular frameworks these days use describe/it syntax (definitely my fault, and I’m sorry) and “describe AThing, it updates the count when …” reads better without should everywhere.
My test names answer the question “what does the software do when it’s working as described?” And “should” doesn’t answer that question: it doesn’t should do something, it does something.
I should (ha!) elaborate. Basically, “should” is language that has overstayed its welcome. should was originally used in place of test because we were trying to get away from “test-centric language” where it caused confusion. So we made test runners that ran should_* instead of test_*. should is a better word than test when people are getting hung up on the word “test.”
But now we have better tools and language to write these things, and the word “test” doesn’t exist in many of the frameworks. Where there is no test there ought not be should. Where this is test, should is probably better. But we have better ways of writing this stuff in most languages now.
(I’m also less convinced that the word test is bad than I was in the early 2000s.)
I kinda miss
1.should eq 2
though, it’s nostalgic for me :)That was a nightmare to maintain, and only existed because of … a bad suggestion that I ran with. :D
Still, it was (at least for me) a groundbreaking new way of thinking about tests and how to do them. When I was doing Rails back in the day, RSpec felt like it really expanded my mind. Cheers to you for having advanced software engineering in a meaningful way!
It depends a lot on test harness DSL.
Some read better than others. Other than that, I consider “should” a bit redundant. I prefer to be more firm in my assertions. “This method does this thing” instead of “this method should do this thing”.
test libraries that provide
it
syntax play well with this approach.Mocha (Javascript):
RSpec (Ruby):
I really like Nim’s unittest library. Simple, but powerful.
I love little tricks like this where a particular bit of language can help you write readable code. Another one I really like is making sure commit messages complete the phrase “if merged, this commit will…”.
Better:
This commit adds the ability to delete users.
Even better:
Adds the ability to delete users.
I think you misunderstood @dstaley’s comment. They’re not suggesting you write “If merged, this commit will add the ability to delete users” as the commit message, but “Add the ability to delete users”, using the imperative mood.
What you wrote is also good, I think, and it’s down to personal taste whether you want the imperative mood (”Add the ability to delete users”) or present tense (”Adds the ability to delete users”).
Ah, my bad. He’s proposing the tpope git message style, which is what I use. I misread it as advice for a verbose message body.
Just wait until someone decides all commit messages need to be stories.
“As a committer on this project, I want to add the ability to delete users, so I can…”
Yes please, great rule. I work with a group that enforces religious use of rebasing and useful commit messages in PRs. We don’t allow GitHub to do squash and merge. We follow your “if merged…” completion style, it just wasn’t defined in those terms. Well stated. So much nicer than what I’ve seen previously.
I’m a big fan of
test_<name>_with_<inputs>_<behavior>
like test_abs_with_negative_returns_positiveThis is fine for unit-tests, but once you move to integration tests you may find this schema limiting.
Eh,
test_login_page_with_successful_login_redirects
,test_compiler_with_recursive_function_emits_tailcall
, etc. It’s a bigger unitThat implies that the only time you ever look at a test is when it fails. I spend a lot of time trying to navigate test organization to write new tests and review code. There’s no stack trace, so the tests just look jumbled together.
Test organization is one of the most frustrating topics, to me. It seems that every language/framework has a different approach to it, none are perfect, and it’s impossible to aggregate all the good ideas, due to technical difficulties. For example, there’s good aspects to the author’s idea, but I can’t use it in
pytest
because test names have to start withtest_
. I’ve largely broken down to using doc strings to explain tests, and I don’t love that that’s the best I can come up with.Yes. I haven’t seen satisfactory solutions either. One thing which helps a bit are explicit coverage marks which sometimes help you to identify where tests for specific area live.
Oh, the coverage marks concept seems interesting!
FWIW this is configurable.
Jeff Forcier (the author of Invoke and the maintainer of Fabric and Paramiko) wrote a plugin for Pytest that lets you specify tests in this more literate fashion as well: https://github.com/bitprophet/pytest-relaxed
It’s not quite as expressive in this regard as rspec is, but it makes for nice readable test output when it’s the right model for your tests.
oh wow, i’ve wanted this so badly. installing immediately. between this and this, my comment has been a great use of time as i use both rust and python :)
Completely agree. The idea tests should have fixed prefixes/suffixes is outdated.
[Comment removed by author]
Personally I’m a fan of given-when-then.
Naturally, by default Rubocop will shout the opposite at you for your RSpec tests, because Rubocop wouldn’t be Rubocop if it didn’t have absolutely nonsensical defaults.
2012s called, they want their rspec back.