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

The options are:

1. this PR
2. make all unittest-based tests end with unittest.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 you import 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 like micropython -m unittest.

IIUC that way will run the tests even if you import 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 read unittest.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 the if __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 a main 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 the if __name__ == '__main__': pattern.

I do like how clean unittest.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 use import __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 by run-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.