-
Notifications
You must be signed in to change notification settings - Fork 63
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.
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
Add "Guardian" test anti-pattern #358
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: "dangerous" not "dangrous".
Sorry. Still working out how this "review" stuff works... :) |
@danon Have you got a clearer example of this? As far as asserting that something doesn't happen is concerned, it seems perfectly fine to assert that something doesn't throw, for example. |
Co-Authored-By: John <50244516+johndotpage@users.noreply.github.com>
What I mean is // given
void shouldNotAppendExtensionForBlocked() {
// given
Resource r = new Resource("/content/docs/file.txt", "some word");
r.makeWrittable(false);
Creator c = new Creator();
// when
c.append("file.txt", "");
// then
assertEquals(r.get(), "some word");
} How are you going to check if this test is not a Liar? Normally, you would comment the Second question: how can you be sure your assertions are correct?
|
@danon I'm not sure I follow (maybe I've missed the point of the code). If the desired behaviour is that appending an empty string doesn't change the file at all, then that test has some value. Not a lot, sure. But some. (It's just an example after all, right?) I would probably rewrite the test (and the code), but it would still be a specification of desired behaviour. void appending_an_empty_string_to_a_text_file_does_not_change_the_files_contents() {
Text file = new Text("/content/docs/file.txt", "some text");
file.append("");
assertEquals(file.contents(), "some text");
} And now, if appending an empty string added "empty string was appended" or else threw an exception, then this test would catch it. |
Let me know If you approve another anti-pattern? :)