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

mock_server_matched returns true when mock_server_mistmatches returns errors #107

Closed
adamrodger opened this issue May 27, 2021 · 11 comments
Closed

Comments

@adamrodger
Copy link
Contributor

I'm writing the Rust FFI interop for PactNet and I've found that mock_server_matched returns true even if there are mismatches returned from mock_server_mismatches. Unfortunately I can only really demonstrate this using the C# code for the PactNet work rather than with proper repro steps:

image

I've also tried it the other way around, only calling mock_server_mismatches if mock_server_matched returns false, just in case I'm somehow resetting the internal state by grabbing/freeing the mismatches string, but mock_server_matched seems to just always return true so I never end up pulling the mismatches string.

@mefellows
Copy link
Member

So this is what the current (beta) Golang FFI handler looks like:

Screen Shot 2021-05-27 at 8 36 28 pm

And where it's used in the actual testing process itself:

Screen Shot 2021-05-27 at 8 35 19 pm

I marked the TODO to find out why. Now that I'm contributing more to the Rust core, I'll see if I can diagnose and fix.

@mefellows
Copy link
Member

tried to repro this locally - if you could share the test pact setup you have (the request/response you're trying to test) I'll see if I can make it fail.

I do recall the problem (hence the comment) but it seems to be working. So it might be some more nefarious bug.

@mefellows
Copy link
Member

But to be honest, aside from getting a JSON string, all mock_server_matched does, is look at the # of mismatches and return true if there are 0 mismatches and false otherwise. I'm happy to skip that altogether personally.

@adamrodger
Copy link
Contributor Author

Yeah cool, so you think it's probably better to consider the verification failed if the mismatches string is non-empty, and not call mock_server_matched at all?

@mefellows
Copy link
Member

Ron might have other views, but for my needs, I get the result and related data in a single call. Looking at the code, there doesn't appear to be other benefits in doing that call (i.e. there are no state mutations).

@uglyog
Copy link
Member

uglyog commented May 27, 2021

mock_server_matched is there as an optimisation to avoid the string allocation for the JSON result where there are no errors. It is not necessary to call it.

@mefellows
Copy link
Member

Makes sense, thanks. I can't reliably reproduce the above problem yet.

@uglyog
Copy link
Member

uglyog commented May 28, 2021

I've run the C test example https://github.com/pact-foundation/pact-reference/tree/master/c/consumer-verification and that is getting the correct values from the FFI functions. That test will fail if mock_server_matched returns true when there are failing results.

This test expects mock_server_matched to return true

$ src/consumer-verification basic ../../rust/target/debug/libpact_mock_server_ffi.so
This is consumer-verification 0.0.0.
Running basic pact test
Mock server started on port 35407
Executing request against http://localhost:35407/mallory?name=ron&status=good
*   Trying 127.0.0.1:35407...
* Connected to localhost (127.0.0.1) port 35407 (#0)
> GET /mallory?name=ron&status=good HTTP/1.1
Host: localhost:35407
Accept: */*

* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< access-control-allow-origin: *
< access-control-allow-headers: *
< access-control-allow-methods: GET, HEAD, POST, PUT, DELETE, CONNECT, OPTIONS, TRACE, PATCH
< access-control-expose-headers: Location, Link
< content-type: text/html
< content-length: 28
< date: Fri, 28 May 2021 00:17:47 GMT
< 
* Connection #0 to host localhost left intact
"That is some good Mallory."

OK: Mock server verified all requests, as expected

This test expects mock_server_matched to return false and mock_server_mismatches to return some results

$ src/consumer-verification error ../../rust/target/debug/libpact_mock_server_ffi.so
This is consumer-verification 0.0.0.
Running error pact test
Mock server started on port 38073
Executing request against http://localhost:38073/?test=hi
*   Trying 127.0.0.1:38073...
* Connected to localhost (127.0.0.1) port 38073 (#0)
> PUT /?test=hi HTTP/1.1
Host: localhost:38073
Accept: */*
Content-Type: application/json
Content-Length: 47

* We are completely uploaded and fine
* Mark bundle as not supporting multiuse
< HTTP/1.1 500 Internal Server Error
< access-control-allow-origin: *
< content-type: application/json; charset=utf-8
< x-pact: Request-Mismatch
< content-length: 528
< date: Fri, 28 May 2021 00:18:12 GMT
< 
* Connection #0 to host localhost left intact
{"error":"Request-Mismatch : Request { method: \"PUT\", path: \"/\", query: Some({\"test\": [\"hi\"]}), headers: Some({\"accept\": [\"*/*\"], \"content-type\": [\"application/json\"], \"host\": [\"localhost:38073\"], \"content-length\": [\"47\"]}), body: Present(b\"{\\\"complete\\\": {\\\"body\\\":123457}, \\\"body\\\": [1,2,3]}\\n\", Some(ContentType { main_type: \"application\", sub_type: \"json\", attributes: {}, suffix: None })), matching_rules: MatchingRules { rules: {} }, generators: Generators { categories: {} } }"}

OK: Mock server did not match all requests.
[{"method":"PUT","mismatches":[{"actual":"[\"hi\"]","expected":"","mismatch":"Unexpected query parameter 'test' received","parameter":"test","type":"QueryMismatch"},{"actual":"123457","expected":"123456","mismatch":"Expected '123456' to be equal to '123457'","path":"$.complete.body","type":"BodyMismatch"},{"actual":"{\"body\":\"123457\"}","expected":"{\"body\":\"123456\",\"certificateUri\":\"\\\"http://...\\\"\",\"issues\":\"{\\\"idNotFound\\\":{}}\",\"nevdis\":\"{\\\"body\\\":null,\\\"colour\\\":null,\\\"engine\\\":null}\"}","mismatch":"Expected a Map with keys body, certificateUri, issues, nevdis but received one with keys body","path":"$.complete","type":"BodyMismatch"}],"path":"/","type":"request-mismatch"}]

@uglyog
Copy link
Member

uglyog commented May 28, 2021

You can see here it is actually testing that condition: https://github.com/pact-foundation/pact-reference/blob/master/c/consumer-verification/src/main.c#L178

There must be something else wrong

@adamrodger
Copy link
Contributor Author

So I think I know why this happened now, and it's a bit of an open question whether it should return true or not in that situation (I think it should return false fwiw).

If you set up no interactions (which was happening for an unrelated reason) and then make a request, you'll get an error back because of the unexpected request. When you call mock_server_mismatches then you'll get back the details of the unexpected request in the string, but mock_server_matched will return true.

Those functions are so similarly named that you'd definitely expect false if you get back an error string.

@uglyog
Copy link
Member

uglyog commented May 30, 2021

This is still confusing to me. mock_server_matched returns the results of mock_server.mismatches().is_empty(), while mock_server_mismatches returns

let mismatches = mock_server.mismatches().iter()
            .map(|mismatch| mismatch.to_json() )
            .collect::<Vec<serde_json::Value>>();

How can the former return true (collection is empty) while the latter returns the unexpected request by transforming the collection into JSON.

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

No branches or pull requests

4 participants