-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
Javascript Testing section: contradiction. #29508
Comments
While I am not a professional on testing, I think I might be able to explain some of the confusion. It is implied that that the One of the most significant reasons is that testing every private sub-function violates access protection. You need to access functions in order to test them, but private functions should not be accessible. To get 100% granularity, every function should be public, which probably is not a best practice. |
Thank you. this is a very good explanation but it is not clear from the course. It makes sense that we cannot test private functions otherwise they would be public but this also means the function we're testing can be working perfectly but fails some tests because some private function is broken. |
You are welcome. If the maintainers agree with my explanation, I can open a pull request to clarify things up. |
Thanks for filing this issue and the discussion so far!
While yes, private functions would by definition not really be testable, I don't know if I would say that's the key part of it. The real crux of it is that when unit testing we shouldn't care about internal implementation details of the unit, because if we do it makes it way more difficult to refactor or extend that unit later on. We should only care about the behaviors as they're visible from outside of the unit, since at the end of the day that's all that really matters.
I'm not sure I understand what you mean here, since if it's failing tests (and they're good tests) that would mean it isn't working perfectly wouldn't it? On the broader topic, there's one particular assignment from the "more testing" lesson a bit after this one (number 5, the talk by Sandi Metz) that really digs into what to test which I think would clear this confusion up. We moved it from the first testing lesson because we felt that was a bit too much info at once. I wonder if we should just add a note to the testing practice lesson that says "this is complex and x and y might feel like a contradiction, but that will be made clearer soon"? |
Thank you for the further explanation! I couldn't have phrased it better myself. While I had some intuition, I believe that "testing a black box" better explains why some functions should not be tested.
I will be glad to open a pull request on this suggestion if that is ok |
Checks
Describe your suggestion
In the lesson project: testing basics the point 4 of the assignment says as follows:
For this one, you may want to split the final function into a few smaller functions. One concept of Testing is that you don’t need to explicitly test every function you write… Just the public ones. So in this case you only need tests for the final caesarCipher function. If it works as expected you can rest assured that your smaller helper functions are doing what they’re supposed to.
In the next lesson, More testing the following is said:
An important basic concept in testing is isolation. You should only test one method at a time, and your tests for one function should not depend upon an external function behaving correctly - especially if that function is being tested elsewhere. The main reason for this is that when your tests fail, you want to be able to narrow down the cause of this failure as quickly as possible. If you have a test that depends on several functions, it can be hard to tell exactly what is going wrong.
It looks like a contradiction to me. Can someone clarify and correct it?
Path
Node / JS
Lesson Url
https://www.theodinproject.com/lessons/node-path-javascript-testing-practice
(Optional) Discord Name
aquilaruspante
(Optional) Additional Comments
No response
The text was updated successfully, but these errors were encountered: