@projectgus@andrewleech@stinos do you have any preference for this fix?

The options are:

1. this PR
2. make all unittest-based tests end withunittest.main(__name__)

Maybe option (2) is cleaner, because then it doesn't matter what the module is imported as?

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.57%. Comparing base(3c1722e) to head(e323da7).
Report is 1 commits behind head on master.

@@           Coverage Diff           @@##           master   #16433   +/-   ##=======================================  Coverage   98.57%   98.57%           =======================================  Files         164      164             Lines       21349    21349           =======================================  Hits        21045    21045             Misses        304      304

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

I prefer the way in this PR, it seems "principle of least surprise" that a Python file executed by the test runner is called__main__ in all cases. We can also hopefully some day move the implementation into mpremote so you can run code via .mpy there, too? (This has been tried by a few folks previously, my own attempt was#8744).

Maybe option (2) is cleaner, because then it doesn't matter what the module is imported as?

IIUC that way will run the tests even if youimport the test module from a REPL to inspect the contents or manually instantiate the test class, wouldn't it?

I prefer the way in this PR, it seems "principle of least surprise" that a Python file executed by the test runner is called__main__ in all cases.

Yes and no. I would think that it doesn't really matter what module a test is imported as, it should just run. Up until now that's been the case, tests just run no matter what. It's only with unittest that things have changed, because of the bit at the bottom of the unittest:

if__name__=="__main__":unittest.main()

Maybe it was an oversight by me to require this code... it's not like these tests are ever going to be imported and used as a module, right? You would always expect them to just run their test. We aren't using auto-discovery here likemicropython -m unittest.

IIUC that way will run the tests even if youimport the test module from a REPL to inspect the contents or manually instantiate the test class, wouldn't it?

Right, if you import the unittest-based test from the REPL then, currently, it won't execute. But if we changed the last bit to readunittest.main(__name__) then it would run when imported.

Is there anything which might rely on__injected_test ?

Maybe it was an oversight by me to require this code... it's not like these tests are ever going to be imported and used as a module, right? You would always expect them to just run their test. We aren't using auto-discovery here like micropython -m unittest.

Even though I kinda dislike theif __name__ == "__main__": unittest.main(), construct it's a very standard way to write tests and I'm leaning towards sticking with that. Because at least for me it's not uncommon at all to run/debug a bunch of unit tests with CPython, so then it's better if they're fully compatible with it to cover all scenarios.

Which in this case would mean for example: editor plugins supporting unittest use discovery to then allow you to run/debug a single test in a file. Pretty sure that's only going to work if the file can be imported as a module.

My first thought was why is it currently called__injected_test in the first place and not just__main__; TIL micropython already has amain built-in. Side thought,__main__.__dict__ could be an alternative location to place objects you don't want garbage collected, similar to a root pointer.

Back on topic, I think the way presented in this PR is best. The__injected_test "module name" is an arbitrary implementation detail, set and used in the same section of commented code that "renames" it to__main__. Sure it's a little hacky on the face of it but it's self contained and for a specific purpose to work around the fact that__main__ is already taken.

I definitely think it's worth keeping theif __name__ == '__main__': pattern.

I do like how cleanunittest.main(__name__) looks but it's a non-standard pattern that might confuse new users and be one extra thing that needs changing when trying to port existing cpython modules. I think keeping things that be developers see as "standard" as possible will help get more people writing tests.

Speaking of confusing people, the one negative side effect I foresee is if said unit test developer tries to useimport __main__ in their tests to get access to the "top test file" but hopefully that's not a common pattern, I can't say I've seen it myself.

Is there anything which might rely on__injected_test ?

No, that's a private internal name used only byrun-tests.py.

My first thought was why is it currently called__injected_test in the first place and not just__main__

I did try changing__injected_test to__main__ and it didn't work, because the built-in__main__ overrides it.

Thanks all for the quick and useful feedback here. Let's go with what's in this PR.