-
-
Notifications
You must be signed in to change notification settings - Fork 783
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
09-defensive.md: Incorrect Error blocks for some assert statements #765
Comments
@Iain-S you are correct, running that |
Yes I like your solution. Thanks. |
I concur. It looks like someone did Thanks for spotting this, @Iain-S! |
@Iain-S would you be willing to submit a PR with your proposed solution? |
I would be very willing to submit a PR to FIX this issue which I've also noticed while reviewing the material in this section. It seems the issue is quite old (still referred to as On this note, in the interest of the discussion, I do believe that both options mentioned by @maxim-belkin and @Iain-S would be compliant with the initial TDD cycle, and both having pros and cons . I am going to try to explain in more details what my thoughts were on this, so we could discuss which solution would be most ideal for an update to the material. A) Changing the first This solution would be great for many reasons: it perfectly links to previous episode's material (a reference link to the That being said, a question now arises: "Where should we go from here?":
This again would mean minimum impact on the current content, but it would slightly violate the original TDD mantra: "Write just the code required to pass the test".
[*] Small side note: In the episode it is reported three test functions for range_overlap:. B) A completely different approach would be keeping everything as it is with the As a matter of facts, this is what I would normally do in real coding, being also a more Pythonic approach to the matter. The downside would be having to explain the In fact, a TDD initial cycle in other languages (e.g. Java) would start similarly to On a similar note, another feedback I'd like to report pertains the strategy used in the episode to introduce the TDD practice. Current approach starts with writing a whole bunch of test cases first, and then an implementation of the function which aims at accomplishing // passing all those tests at once. Whereas, to the best of my knowledge, TDD is meant to be much more incremental than that, and aimed at developing one feature at a time (as usually covered by a single test at the time):
On this note, the A unit test function (esp. when developed w/ TDD) should either cover one feature at a time (with single or related tests), and have a self-explanatory name. This would also serve the purpose of documenting what features or cases are actually covered by tests. Therefore to conclude - apologies for the very long comment - I would really really love to hear from you on those few points I've made. Any feedback or suggestion will be very much appreciated. Also, I would love to work on this, and submit my PR to contribute to this episode :) Looking forward to receiving from you. Thanks very much! |
@leriomaggio I certainly don't mind if you want to fix this issue. Somehow, I never got around to it. You've clearly given the TDD section a lot of thought. The simplest approach, as you've mentioned, would be to either stub the function before the call to Personally, I got a bit of an "aha!" moment the first time I saw someone write the test before they even declared the function. I support being more incremental in how the tests are written. There is a slight complication, though, in how you test that the function exists. I would normally do something like this with
but the section begins by using
or
or
Personally, I think the episode section would be better re-written so that it goes:
I would introduce "red, green, refactor" as an easy way to remember that you should always start with a failing test case. I think it would be fine to take out the extra assertions that are there at the moment, they are probably a distraction. |
If anyone plans to do a rewrite of the |
Thank you @ldko for your feedback. Yes, I would be very happy to work on a revised version of the episode. Re Re @Iain-S : I agree with everything you said in your comment 💯 Thank you.
Exactly! My idea was indeed aimed at focusing the episode more on the TDD practice, rather than on testing in general, as it seems to be at the moment. |
Problem description
In the episode on defensive programming, under the "Test-Driven Development" heading, we use assert like so:
The error block below this code shows an
AssertionError
. In fact, we get aNameError
as range_overlap is not yet defined. This happens again a little further down the page.Proposed solution
Run the Python
assert
statements under the TDD heading. Copy theNameError
outputs and use them to replace theAssertionErrors
, where appropriate.The text was updated successfully, but these errors were encountered: