-
Notifications
You must be signed in to change notification settings - Fork 2
Implemented Github Actions for Mac and Added Mac Compatibility #6
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
base: master
Are you sure you want to change the base?
Conversation
steel-bucket
commented
Apr 1, 2025
- Added Mac compatibility
- Added Actions which builds and tests the code for Mac.
- For Mac it was simple, as it's unix based, I just had to make the test_continuous_output_does_not_timeout time to 5 seconds instead of 3.
src/lib.rs
Outdated
| result.duration < Duration::from_secs(3), | ||
| "Duration should be < 3s" | ||
| result.duration < Duration::from_secs(5), | ||
| "Duration should be < 5s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow not running without it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I do some research?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I do some research?
Yes, we can't merge something without understanding what's going on :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cfsmp3 Okay, so I did some research. First I thought that the problem was that "sleep" was taking slightly more time in macos. So I tried with bash and zsh, but they didn't work. Then I tried to perform the timing before the draining, that didn't work. Then I started looking for the functions that may be different for Linux and Mac in the code and not just in the tests. I thought the libc::setpgid would be the difference so I tried replacing it with nix::unistd::setpgid, but that didn't work either, and debugging showed that the duration is unchanged after setpgid.
So now, the thing that I'm almost sure of is that the slight time change originates from the killpg function, so I guess Mac takes slightly more time killing the application.
I've modified the tests for asserting different duration for Linux and Mac. Please suggest any changes.
Thank you.
7ec33b1 to
c3e48e5
Compare
a211626 to
2f32614
Compare