-
Notifications
You must be signed in to change notification settings - Fork 234
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
Wrong result validation when publishing pacts? #486
Comments
@mefellows this looks like it could be an issue with the FFI |
You need to supply a valid PactFlow token (hence the TL;DR - it's not a Pact .NET issue. |
I know how to fix problem with authentication. This issue was about result validation and not authentication issue. I don't want to have situations when there is error but test has successful status. |
I see. The publishing of the verification is unsuccessful, but the tests passed. The overall result should be a non-zero exit status (which presumably is happening). How would you expect the API to behave here? I guess a clearer error message could help, but I'm wondering if bubbling this to the client libraries (in this case, Pact .NET) is worth doing. How would you both convey the tests passing, but the process failing? Are you having this problem locally or on CI? In case it's not obvious, you shouldn't be publishing from your local development environment, and so this should be a CI problem. If it fails in CI, the only acceptable thing to do is fail the overall process (so you don't have a false sense of security). See also https://docs.pact.io/diagrams/ecosystem for background on the Rust core and FFI. |
From the library perspective I'd want the FFI to return a non-zero status code so I could throw an exception like we do with a few other error cases. Then the user is made aware that the test run as a whole has failed even if every individual test has passed. |
The current error handling from the verifier FFI is here, where any non zero response would fail the verifier run: If "unable to publish results" had its own error code then I could throw an exception with that message, for example. At the very least it could reuse the code for verification failure and then the user would check the logs to see why. |
@uglyog what do you think? |
This is not a trivial change. The result is the result of the verification. The verifier does not propagate the status of whether it was able to successfully upload the verification results or not. |
Perhaps the publish verification results should be an independent function to the verification rather than mixing the concerns? I can see the challenge with that (you would probably need to store a result type, pass an opaque reference back and then use that reference in a separate call). Perhaps just propagating it in the verifier call itself would be sufficient, at least that would result in a broken build and would be more "obvious". |
Do you plan to reopen this issue? |
I've reopened but this requires an FFI change, not a PactNet change. |
I'm using 5.0.0-beta.1 to send data to pact. In logs there is info "Pact verification successful" but in logs
So my test are green but no pacts reached the server. For now I would have to manually analyze logs and react for this. I think this should be catch by pact-net.
The text was updated successfully, but these errors were encountered: