search

magmax / python-readchar

Python library to read characters and key strokes
MIT License
143 stars 43 forks source link

run Linux tests on Darwin as well #99

Open dotlambda opened 1 year ago

dotlambda commented 1 year ago

I made sure they run fine. This change helps us run the testsuite in Nixpkgs.

Cube707 commented 1 year ago

Thanks for the PR, and I am quiete sad to have to turn it down. But the same reasoning as on PR 86 applies here as well.

Enabling tests for more platforms means we must also be ready to handle issues for these platforms and ensure they work in the future.

I already have my hands full handeling Windows and Linux and am not capable/willing to handle OS's I am not familiar with (and don't have running locally...).

This is why darwin and BSD are listed as "enabled, but not supported" and the testfolder is still named "linux" and not "unix"

dotlambda commented 1 year ago

This is why darwin and BSD are listed as "enabled, but not supported" and the testfolder is still named "linux" and not "unix"

That could remain the case. It's just useful for e.g. Nixpkgs to be able to test correctness of this package on Linux as well as Darwin. I assume you'd be happy if we reported an issue or filed a PR if the testsuite ever failed on Darwin?

Cube707 commented 1 year ago

Yes, please open issues and PRs if you find any problems. If they are solvable on the linuxside of the code I will be happy to fix.

Cube707 commented 1 year ago

just to clarify a little more:

currently the library has completely separate code for Windows and Linux, both for the implementation and the tests. This hold a lot of duplicate code in is quite annoying to maintain. This also means that adding support for a new OS requires copieing and modifying all that code again, which is especally hard when I don't have a local dev-system for that OS. Thats why I am currently not doing anything in that regard.

I know this is a controversial point, but for me enabling the tests for a OS means I obligate myself to that OS and should have it working. For this reasons I don't accept this PR at the moment.

Should we ever get to a point where the code is more generalized and the OS specifics can be reduced to a small subset of functions, which are much easier to test and maintain, I would be super happy to enable more OS's.

I am working towards if I find sometime to develop this project (which is rare), but currently I can't give a timeframe and its quite a nightmare to get working reliably even for windwos and linux. 😢

So to sum up: I am happy to work the linux side to resolve problems on darwin, as long as its compatibly with linux.

Specifics that requrie a seperate implementation will have to wait untill offical support can be supported.

Cube707 commented 1 year ago

please keeps this open. This is still a valid PR and I want to make it easy for people to find this and read why this is currently on hold 😄