docs: shard testing content out into multiple files #63
Conversation
Based on this suggestion[1]: > Maybe we should split `testing.md` into different linked sections with > a generic TOC to aggregate everything? So, move the file into a "testing" directory and rename it to "README.md": that way GitHub will show it when you navigate to: https://github.com/liferay/liferay-frontend-guidelines/tree/master/general/testing In the next commit I will turn this into a table-of-contents and split its text out into separate files. [1]: https://issues.liferay.com/browse/LPS-97586
Based on the list suggested here: https://issues.liferay.com/browse/LPS-97586?focusedCommentId=1901126&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-1901126 which was: - Accepted libraries - Anatomy of a test - Examples I didn't add "Examples" yet, because I didn't want to come up with something contrived, nor am I sure I'll be able to find a "perfect" example among our existing tests, but I did add a section on testing UI components because we already have a fair bit of content for that.
wincent
force-pushed
the
wincent/test-split
branch
from
200a346
to
0f2f8c5
Compare
|
|
||
| In software development, we try to balance a number of nebulous concepts like "readability", "maintainability", and "ergonomics". This in turn leads to maxims like "Don't Repeat Yourself" (DRY). | ||
|
|
||
| > In test code, it is **even more important that code be readable and intention-revealing**. Don't worry about duplication in tests: it may actually be a good thing. |
|
|
||
| ### Concrete recommendations | ||
|
|
||
| #### Assert (ie. `expect()`) one thing per `it()` block |
There was a problem hiding this comment.
Should we enforce this? I'm assuming we'd rather leave it as a recommendation only as we'll probably need to deviate from this...
There was a problem hiding this comment.
Yeah, that's my thinking as well. When I originally wrote this I even started the sentrence with "Try to..."
This kind of advice is like "try to keep your functions shorter than X lines" — it's good to bear in mind the benefits of short functions, but if you slavishly chop all of your code up into bite-sized pieces then sometimes you're going to actually make it worse. So it's good to leave some things up to individual judgment.
There was a problem hiding this comment.
Im not a huge fan of this rule. Typically because I like to assert before and after whatever action I take. For example if I expect something to happen when I click a button, I typically assert it didnt happen before the click and then assert it did happen after the click.
There was a problem hiding this comment.
@bryceosterhaus: It's not a "rule" though. We can soften the language to make it clear that it is not "one-assertion-or-die" (please feel free to do that, or to add some sentence(s) explaining your use case).
A pre-and-post assertion is obviously quite a sensible thing to do; in fact you could argue that your pre-and-post expect calls are actually just two halves of a single assertion about a transition between two states. RSpec (a Ruby BDD library) used to have matchers explicitly tailored for this style of the form:
expect{Counter.increment}.to change{Counter.count}.from(0).to(1)
expect{Counter.increment}.to change{Counter.count}.by(2)Would be pretty easy to make a JS/Jest equivalent, I'd think:
expect(() => button.click()).toChange({
value: () => component.counter,
from: 0,
to: 1,
});
expect(() => button.click()).toChange({
value: () => component.counter,
by: 2,
});
// or something else equivalent; this is just one sketch...The main goal of this recommendation is to avoid the kind of insanity in the counter-example in the docs where you have a test that doesn't clearly specify the behavior at all because it is asserting half-a-dozen things at once.
There was a problem hiding this comment.
Sounds good to me. I figured that is what you were getting at but just wanted to make sure and clarify.
general/testing/README.md
Outdated
| }); | ||
| ``` | ||
|
|
||
| This (contrived) example is one where your intuition may be to "DRY" up the code, but consider the position of the person who reads a report of a "test failure on line 123" and now has to figure out which iteration of the loop. It's better for them if we spell out each example alone. |
There was a problem hiding this comment.
nit: maybe add a link to the DRY section?
There was a problem hiding this comment.
Something like this?
I'll merge this with that, and if I misunderstood what you meant please feel free to edit.
One to an external document saying what "DRY" is, and another as a back-reference to the prior discussion. In response to this comment here: #63 (comment)
As requested here, this PR splits out our testing documentation into multiple files and expands it a bit.
This PR is just a start — there is still a lot more that could be said — but it gets the ball rolling.