CleanCut / green

Green is a clean, colorful, fast python test runner.
MIT License
786 stars 74 forks source link

Hyphens in module names cause them to be skipped #136

Closed mairsbw closed 7 years ago

mairsbw commented 7 years ago

I'm not certain if this is intentional behavior or not, but I only noticed it when I started playing around with pytest, which does not have this behavior.

I had a module named test_de-vignette.py which was silently ignored by green. Renaming it to test_devignette.py causes it to be run (and fail, since I hadn't been running it all this time!).

CleanCut commented 7 years ago

Using hyphens in module names is not a good idea, and leads to many problems. (For example, just try import test_de-vignette).

I don't plan on adding support for hyphenated module names.

mairsbw commented 7 years ago

While that's fine, I think it's a misfeature to ignore them entirely. pytest doesn't, which is how I found this in the first place. While it actually runs the tests, maybe green can warn about modules it finds that have hyphens, or lists them as Skipped? I've written Python for many years and never knew about the "badness" or hyphenated module names. I'm sure many other people will make this mistake, and the worst part is, that it's a silent failure. This is one of the problems with frameworks automatically determining what to run, that there can sometimes be a disconnect between how the authors and the tools understand what code/data should be run/processed.

CleanCut commented 7 years ago

The problem here is that I rely on Python's own unittest module to do the actual test discovering (part of the Traditional feature). So it is the built-in module that is ignoring it. Pytest wrote their own discovery code, so they can do what they want.

To change behavior, I would either have to monkey-patch every version of python that I support (not pretty), or write my own discovery code (ouch). I'm not really interested in doing either when Python itself doesn't consider the case.

Of course, you or someone else wanted to do the work and submit a patch, I would be happy to review it and accept it (as long as it's not crazy code that's going to be difficult to maintain).

mairsbw commented 7 years ago

Yep, what you said makes sense. It's too much work to support for such a small bug that shouldn't really happen in practice. Thanks for explaining it more fully.

CleanCut commented 7 years ago

No worries. I personally wish that they would improve unittest and fix issues like this as versions went along, but there is something to be said for stability (nothing ever changes) as well.