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

Had issues on windows - fixed them + added custom descriptors support to Shell #77

Merged
merged 7 commits into from
Feb 11, 2023

Conversation

shlomohass
Copy link
Contributor

Summary:

  • Tests were failing on windows mainly because 'phpunit' was firing multiple attempts to read / write to the same files without locking.
  • Fixed Colors tests which were comparing \n to PHP_EOL - which is not reliable and fails on windows.
  • Safer unlink in tests to avoid warnings.
  • Shell was using a NUL file descriptor of stdout and stderr - changed that and added support for "custom" pipes declaration passed to execute().
  • Improved isWindows() in Shell
  • Safer reading, writing and closing of pipes in Shell - checks to see they are a viable resource.

Tested it again passing all tests on windows and linux... ✔️
🚦 PLEASE - add a new release, 1.2 is the latest and doesn't include the $env bug fix which was merged later.

🚀 Great project - lightweight, clean and neat

@shlomohass
Copy link
Contributor Author

Fixes #76

@adhocore
Copy link
Owner

adhocore commented Feb 7, 2023

thank you very much. what do you think, should we add some cicd that runs on windows?

@shlomohass
Copy link
Contributor Author

@adhocore cicd on windows will be great. Like it or not windows should be supported 😄

@adhocore
Copy link
Owner

thank you very much, looks great 💯
(i assume you tested this changes in win os? 😉)

@adhocore adhocore merged commit 4c2bab4 into adhocore:main Feb 11, 2023
@shlomohass
Copy link
Contributor Author

Yes tested it in win10 11 and server... also tested on nix. Happy to contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants