Collab with tienvx: use ffi #257
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
Lewiscowles1986
commented
May 28, 2022
•
Lewiscowles1986
commented
- Sunset PHP 7.3 #258
- feat: Use Rust FFI #210
Lewiscowles1986
force-pushed
the
collab/tienvx/use-ffi
branch
3 times, most recently
from
3caff99
to
225b3da
Compare
Last release Jan 2021... Has shared objects not shipped with. Goodbye.
Lewiscowles1986
force-pushed
the
collab/tienvx/use-ffi
branch
from
225b3da
to
c8fe4e7
Compare
12 tasks
|
@Lewiscowles1986 this needs a rebase, but would be good to go otherwise, correct? |
|
@mmoll it was working if this what what you are asking. I'll get it rebased and then we'll see if anything has changed. |
|
After rebase this doesn't pass. No clear error message about why.
Are there perhaps undocumented extensions or setup needed? I noticed a lot of amphp in composer.json. Usually that needs threading PHP. In-between runs I am deleting the |
|
|
The runner gets diff --git a/src/PhpPact/Standalone/Runner/ProcessRunner.php b/src/PhpPact/Standalone/Runner/ProcessRunner.php
index bcd8325..4b60ed5 100644
--- a/src/PhpPact/Standalone/Runner/ProcessRunner.php
+++ b/src/PhpPact/Standalone/Runner/ProcessRunner.php
@@ -129,7 +129,7 @@ class ProcessRunner
$logger->debug("Exit code: {$this->getExitCode()}");
if ($this->getExitCode() !== 0) {
- throw new \Exception("PactPHP Process returned non-zero exit code: {$this->getExitCode()}");
+ throw new \Exception("PactPHP Process returned non-zero exit code: {$this->output}");
}
Loop::stop(); |
|
I'll be honest. Most of my problem with this is that I'm not very interested in getting to the bottom of it. The FFI library should be less flaky than the sub-process approach taken by the PHP code now, but this PR doesn't remove all the sub-process nonsense, nor make it more clear about errors. I ran the command that is reporting as failed. It's exit code is RE: the edit to get output, I think going a few lines up and placing it within the debug might be closer to the intent. Filling exceptions with intricate details is a smell, that will only make it harder to deal with logging and troubleshooting. At least in an explicit debug log message it should never reach logs off a developer computer. Output of the command (formatted "pretty" JSON)
Gets the error message This reads like it's a downstream system failure, which makes this test not fit for purpose. |
|
Utterly abysmal... Someone never heard of stable URL's Command output
|
|
Hey dude, Merged master into this and set off a build on another branch https://github.com/pact-foundation/pact-php/actions/runs/3008301582 I needed to download an arm64 version of the Your work really is appreciated, especially as you have hit a few roadblocks |
|
Hey @YOU54F I've encountered this issue recently too. It seems a fair problem as this lib takes the responsibility to download the binaries. (which I think is wrong). |
|
next try: #270 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.