Today, I found this in our upstream repo. How the fuck does it even get merged?
No use of tempfile, instead of idiomatic it's idiotic. I have changed that system, now I am hoping i don't have to refactor thia shit.

It looks like its a test to see if it can write a file and clean up afterwards.

But I also cannot spot the big error.

Well self.assertEqual should be self.assertTrue and why there is flag variable for everything.
flag = False
If f():
flag = True
Can be rewritten as flag = f()
And here he is creating file for testing in project directory instead of temporary directory so if some test fail before it hits os.remove it won't do clean up. So, one has to clean that dump by himself.

I found a lot of dependency in the code.

It use "with self.assertLogs().... as cm", that "cm" have "cm.output[0]" which store the file name.

It will be great if the output file name is unique every time otherwise it will be difficult to know why test fail since the condition may be true due to other "test".

There is also subtle problem in that "Output stored at" should be prefix of cm.output[0]. If we decided to store use other prefix , the test will fail. It will be great if cm.output[0] return empty string if error occur so we can just used

"if len(cm.output[0])>0"

The main problem I have is that the "assertLogs() cm" and "output engine" are too connected in a way which make testing difficult.

In conclusion , we just need to know what @-pthread have problem with it.

Edited : Forget that I said anything , he posted his problem above.

@-pthread

Well Is there anyway you can change the output path of output engine to "temp" folder?

My main problem is with clean up logic. Here, if logic for output_file raise some unexpected error or assertion fail then os remove won't get to run and an extra file will get generated.
Always this kind of logic should be in teardown.

@mr-user during test setup, one can create temp directory and change current working directory to that temp directory so any file generated by output_engine will be dumped into that temp directory and in teardown we can just remove that temp dir.

@mr-user I know it's bad to assert logging messages if it was upto me I just test original functionality that file is being generated.

You must be a delight to work with.

@-pthread I love how you are focusing first and foremost on not using the utility assert.True function, and about how the Boolean variables assigning is not optimized. Then comes the REAL criticism.

This is a surefire way to get the message lost in a code review. Very non-senior of you.

@mellowmymind What do you expect, this is devrant not stack overflow or a PR.

@jespersh Well, Ex. The if os.path.isfile() line should not be an if statement, it should just assign the contains_filename variable.