Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

run Linux tests on Darwin as well #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dotlambda
Copy link

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

@Cube707
Copy link
Collaborator

Cube707 commented Feb 24, 2023

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"

@Cube707 Cube707 added the out of scope this is currently not managable (but may be in the future) label Feb 24, 2023
@dotlambda
Copy link
Author

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
Copy link
Collaborator

Cube707 commented Feb 26, 2023

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
Copy link
Collaborator

Cube707 commented Feb 26, 2023

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.

@dotlambda dotlambda closed this Feb 26, 2023
@Cube707
Copy link
Collaborator

Cube707 commented Feb 26, 2023

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 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
out of scope this is currently not managable (but may be in the future)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants