The code is not garbage, it's just your highfalutin python opinion makes it so you only ever use list comprehensions or return generators.
For loops in python that return non lazy evaluated lists are fine. Python was never suppose to be an efficient language anyways, grading python based off of this criteria is pointless.
It doesn't matter how snobbish you are on language syntax though. I fed it code and regardless of whether you think it's garbage it did what I asked it to do and nothing else.
Would you prefer the AI say, "this code is garbage, here's not only how to make it unit testable but how to improve your garbage code." Actually we can make the output more unpredictable as LLMs do have a non deterministic seed that can increase the creativity of the answer.
It has wrapped range() with useless code. It has added no functionality, it has not improved testability in any way.
.
Please, take the code it has produced and integrate it into the original function. All it does is replace the range call. That's it. It has absolutely and totally failed at the given task whilst outputting plausible garbage about why it has succeeded.
Let me tell you your mind is going to be blown once you learn about Monads.
A mutable object is functionally identical to a return value if you control the initial state and lifetime of the object. Like you can do in a unit test.
And as I demonstrated in my other comment I 100% retained the semantic structure of the function whilst making it 100% unit testable.
I think you don't understand what unit testability means. It means removing IO and side effects from your code.
How the hell do I test a print function? I take the print function and match it with what? It has no output so how can I test it printed the correct thing? I can't.
I can test a list. I just match it with another list. Making your code unit testable is about segregating IO from logic. Write pure logic where all functions have inputs and outputs and those things can be tested. Your io prints should be small because all functions that do io cannot be fully tested.
IO is pollution. Any output to IO is the program exiting the logical mathematical universe of the program and that output can be verified only by an external entity. Either your eyes for stdout or another process or files or a bunch of other ways.
Unit tests are about internal local tests that touch local functionality and logic. If you want something unit testable it needs a local output and an input and it shouldn't rely on io in it's data path.
I think your complaint here is an example of chatGPT superiority. It understood something you didn't. Well now you know.
Removing the print function from the logic and returning the data is 100 percent the correct move. Do you understand?
Of course you can make the function with a print statement more unit testable without completely changing it's semantics!
You pass in an outputstream and use that as the target for print.
Then your unit test can create its own stream and test the content of the stream whilst production code can pass in standard out.
That way you don't completely change the semantic meaning of the code.
And once again that GPT function is useless. It is identical to list(range()) and it doesn't do what the first function does. Anyone can make anything more unit testable if it doesn't have to do the same thing.
Bro, dependency injection and mocking is the same thing as segregating your function from IO. Your replacing io calls to stdout with io calls to something else. But that doesn't make your code unit testable.
The function is still touching io. You gonna test it with another function that touches io? That defeats the point of the definition of unit testability.
> and doesn't do what the first function does.
Are you serious? You mock your output streams with hacky monkey patching your function ALSO stops doing what it originally does. It's essentially black magic globals that mutate your program... very bad practice.
Chatgpt here just didn't write the obvious io component of the code because it would be freaking pedantic. The full code would include a function that prints lists composed with a function that produces lists. The composition allows part of the program to be testable while leaving the io part of it not testable. For the original program NONE of it was testable.
Your Monkey patching here would be replaced by different io functions. You want to change the output stream? then you change the IO function. Compose the list producer with another IO function. Play type Tetris and you can recompose your list producing function with all kinds of modular io. The point it you separated the core logic away from IO thereby making it more modular and more testable.
None of the io functions are testable via unit tests, that is the point. That is the definition of the most basic form of testing... Unit tests.
You literally HAVE to change your code in order to make it unit testable. If your code is throwing shit to io and retrieving values from io then none of your code is unit testable. You're at the integration test level and at this level things become hacky and more complicated. Your tests not have external dependencies like state, the operating system and you have to run hacks like your monkey patch.
Where ever you work or whatever you've been doing if you haven't been doing what I described then you (and your work buddies) haven't been testing your code via unit tests.
That's fine, whatever works bro. But chatGPT knows the common parlance for testing and unit testing, and it did exactly the correct thing.
Your interpretation of what testing is the thing that is strange and off here.
I'm sorry, I clearly haven't explained myself well as otherwise you would not have wasted a huge amount of text tying yourself in knots based clearly on a mistaken apprehension of what I was saying.
For clarity I reproduce the original function you gave and then I present what the change I am suggesting is
def cool_function(x):
for i in range(x):
print(i)
My change
def cool_function(x, output_stream=sys.stdout):
for i in range(x):
print(i, file=output_stream)
Does it now become clear what I am suggesting? My new function can be used as a 1-for-1 replacement for the old function, no code of the system needs changed as the default value provided to the new variable ensures semantically identical operation without changing any further code. Yet it is now unit testable
So I've made the code unit testable, kept semantics completely identical and not had to worrty about any weird IO concerns that you have. No monkey patching, no weird file IO, no bizarelly re-implemnting list(range(x)).
> I'm sorry, I clearly haven't explained myself well as otherwise you would not have wasted a huge amount of text tying yourself in knots based clearly on a mistaken apprehension of what I was saying.
No need to apologize. This is a discussion. No one did anything wrong.
>For clarity I reproduce the original function you gave and then I present what the change I am suggesting is
This is called dependency injection and it's a valid way of segregating IO away from pure logic. Although this pattern is popular among old school OOP programmers it's getting out of vogue due to the complexity of it all. You used a python trick here of default values, but typically dependency injection changes the function signature and ups the complexity of the code by a lot. Let me show you the full output of the code that chatgpt was implying:
#unit testable code (without using dependency injection tricks)
def cool_function(x: int) -> None:
IO_function(logic_function(x))
def logic_function(x: int) -> List[int]:
return [i for i in range(x)]
def IO_function(x: Any) -> None:
print(x)
def test_output():
assert logic_function(4) == [i for i in range(4)]
Chatgpt only gave you logic_function, because IO_function is sort of obvious.. it's just "print" (I only wrapped print in "IO_function" to keep things clear, typically you won't define that function). But basically the full complete code would be to recompose IO with logic. You now have two components one of which is testable.
As a side note you will see it's actually an improvement to the code. It's simpler, no dependency injection, no confusing function type signature and a much simpler test case. The other thing that must be noted is the modularity.
Making tests unit testable in this way allows for your logic to be portable. What if I want to repurpose cool_function to output it's logic to another function? In your example you don't have the components to do that, it's harder for your case as you'd have to create another component for injection.
In short not only did chatGPT produce A correct answer. But it produced the better answer compared with your dependency injection. That being said your dependency injection is valid BUT you were not correct in saying that chatGPT's answer was worse or incorrect.
3 functions is better. Think about it. Do people write all their stuff in one big function? No. Better to compose higher level functions with smaller ones rather then write one big monolith like you did. The more modular something is the better.
Also IO_function is there for illustration purposes. Technically it's just wrapping print with a name so you can understand the intent. In reality you just use the regular print here without a wrapper, so in actuality only two functions are defined.
>The job of ChatGPT was to make cool_function unit testable. You haven't done it.
It did. By giving it a return value. Just like you did by giving it a new input value.
>You still have cool_function using side effect generating code hitting the actual IO system.
Yeah but one component of cool_function is pure and you can unit test that. Cool function itself can never be tested because it generates no output, you test the unit components of cool function. That's the point of unit tests.
>Genuinely the worst unit test I have ever seen written, on a poor form per line basis, absolute bananas. If you don't understand why [i for i in range(4)] is bad in a unit test and [0,1,2,3] is correct then I need you to walk away from the computer.
Let's just talk about it like adults. Just tell me what exactly about it makes you think it's bad?
Most likely it's some pedantic stylistic philosophy you have? I'm thinking you only want to test literals? Perhaps you prefer [0,1,2,3]? Am I right on the money?
Logic potentially has errors so you don't put logic in your test code. Makes sense, but who cares. For trivial shit it's fine. While in this case the logic in the test is identical to the function, typically 'logic_function' represents something significantly more complex and the list comprehension so I could care less if I'm not following the strictest form of testing. The comprehension is just something akin to an alias shortcut I prefer to use over writing out a massive literal. For the toy example the test is pointless because the logic is identical but typically it's fine to use range as an alias to represent a sequence of numbers.
Someone who strictly follows these stylistic rules without seeing intent or having the ability to bend the rules is just an inflexible pedantic programmer. It's not good to boast about it either by telling other people to walk away from a computer. That's just rude.
That would be fine if the core thing needing unit testing was the data generation/ transformation logic, but just as often as not it's the output formatting too. Did you try asking ChatGPT to write a unit test to confirm that the output is displayed as expected?
>That would be fine if the core thing needing unit testing was the data generation/ transformation logic, but just as often as not it's the output formatting too.
Output formatting touches io. In this case it is no longer a unit test that touches these things. Unit tests by definition test ONLY internal logic and transformations.
It is literally the definition of unit tests.
When you test things like stdout that becomes an integration test and Not a unit test. It requires some external thing or some global black magic monkey patch that changes what print does to do integration testing.
(Btw making print formatting unit testable means segregating the formatting from the print. Produce the string first, test that, then print, because print can never be unit tested by definition)
Typically programmers segregate these levels of testing because unit tests are easier to write. But to write unit tests your code has to be written in a way to cater to it. Often this style of coding actually improves your code it makes it much more modular. The reason is because pure functions that output data can be composed with all kinds of io functions. You can move it all over the place and to different platforms with different forms of IO. Print has no meaning in certain embedded systems so it can't be moved... By segregating the logic out it makes it so I can move the logic without the io baggage.
Chatgpt 100 percent gets the difference that's why it did what it did. I think you and the OP don't fully understand the meaning of unit testing.
Don't take this the wrong way, but just because you don't know this doesn't say anything about your skills as a programmer. But just recognize that this concept is basic and is pretty much something universal among testing.
> Unit tests by definition test ONLY internal logic and transformations
Output formatting is still a type of transformation! The function explicitly takes the numbers and prints them as decimal integers with newlines between each.
A test to confirm that it IS in that format is still a unit test.
BTW I gave ChatGPT the prompt I would give, and I have to say the answer looks pretty good, even if I'm not a Python programmer and it's not the way I'd do it (which would be to change the function to allow passing in an output stream):
class MyFunctionTestCase(unittest.TestCase):
def test_my_function(self):
expected_output = "0\n1\n2\n"
with patch('sys.stdout', new=StringIO()) as fake_out:
my_function(3)
self.assertEqual(fake_out.getvalue(), expected_output)
With a few more prompts I also managed to get it give me this version:
def my_function(x: int) -> str:
output = ""
for i in range(x):
output += str(i) + "\n"
return output
Which I'd argue somewhat changes the code that was originally written, but it's still a pretty decent answer.
There's no doubt there's some impressive stuff going on that it can do such things, the real issue for me is that when I've tried on far more complex functions it's tended to break down (quite badly in some cases).
>Output formatting is still a type of transformation!
I'll quote part of my reply (which you missed):
(Btw making print formatting unit testable means segregating the formatting from the print. Produce the string first, test that, then print, because print can never be unit tested by definition)
Right? Think about it. You want to unit test your formatting, remove the logic from the atomic IO function. Otherwise you can't test it via a unit test because that's the definition of unit testing. I realize that there is formatting that's part of the internal functionality of printf, but really all that means is that funcitonality can never really be unit tested. If you want to test printf, that happens at the integration level... By Defintion.
>BTW I gave ChatGPT the prompt I would give, and I have to say the answer looks pretty good, even if I'm not a Python programmer and it's not the way I'd do it (which would be to change the function to allow passing in an output stream):
It's wrong in this case. Unless you specifically asked it to write unit testable code, what it did here is write a hack that monkey patches the program. It's a huge hack. It didn't write unit testable code, but rather it wrote a integration test that monkey patches stdout, negating any need to make your code "unit testable" no refactoring needed using this method. The entire concept of refactoring code to be unit testable flies out the door in this case as you're just using integration tests to get around everything.
I mean yeah you use the unit test library but is not technically a unit test. It's fine I'm not a stichler for what style of testing is used in practice but what I am saying is that what chatgpt did previously was literally follow my instructions to the letter. It did it exactly 100% correctly. Think about it. I asked chatgpt to make the Code more unit testable. You didn't have chatgpt do anything to the code. You simply changed the test from a unit test to integration test. Huge difference. I mean if your case was the "proper" way then what does it even mean to make code "unit testable" if you're not even touching the code? Like why does the concept of "making code more unit testable" even exist if we're just changing tests to make everything unit testable? Ask yourself this and you'll realize that the only answer is basically what I just told you previously.
I've been writing unit tests for over 15 years (actually longer, but before that they were just throwaway run-once testing stubs). I wouldn't consider what you got ChatGPT to produce to be an adequate rewrite of a function to make it unit testable (and several others in this thread have expressed the same view). Even the "hack" using monkey patching makes for a more actually-useful test.
I'm perfectly aware of what the difference is, thank you. The function you gave to ChatGPT explicitly requests a stream to format integers as decimals, separated by newlines. The version it gave as being 'unit-testable' did not, and hence wasn't a 'factually correct' answer. In some cases that may be perfectly fine, but in others it most definitely isn't.
For loops in python that return non lazy evaluated lists are fine. Python was never suppose to be an efficient language anyways, grading python based off of this criteria is pointless.
It doesn't matter how snobbish you are on language syntax though. I fed it code and regardless of whether you think it's garbage it did what I asked it to do and nothing else.
Would you prefer the AI say, "this code is garbage, here's not only how to make it unit testable but how to improve your garbage code." Actually we can make the output more unpredictable as LLMs do have a non deterministic seed that can increase the creativity of the answer.