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

Some problems with pact specs #177

Open
frvade opened this issue Aug 8, 2018 · 27 comments
Open

Some problems with pact specs #177

frvade opened this issue Aug 8, 2018 · 27 comments

Comments

@frvade
Copy link

frvade commented Aug 8, 2018

Hi

We are coding on ruby (https://github.com/umbrellio) and are looking for the system for testing our decoupled services. In particular we have a number of HTTP APIs. At some point we found Pact and decided to try it out. And we encountered some difficulties using it.

First of all, it's somewhat weird that core ruby implementation doesn't implement 3rd version of pact-specification.

Then we have found some bugs (or we just didn't understand how it works).

For example, when we describe provider state at the provider side and we have an exception in set_up block, it doesn't call the tear_down block. We are using transaction database cleaner strategy, and we can't rollback the transaction because of this behaviour. We would like to use something like rspec around strategy, but it just doesn't exist.

Secondly, we'd like to use an array of regexps in consumer pacts. We tried to use this code:

dates: each_like(Pact.term(generate: "2018-08-03", matcher: /\d{4}\-\d{2}\-\d{2}/)),

and on the consumer side it sucessfully generates the json with:

        "body": {
          "dates": [
            "2018-08-03"
          ]
        },
        "matchingRules": {
          "$.body.dates": {
            "min": 1
          },
          "$.body.dates[*].*": {
            "match": "type"
          },
          "$.body.dates[*]": {
            "match": "regex",
            "regex": "\\d{4}\\-\\d{2}\\-\\d{2}"
          }
        }

As we can see, there is a matching rules for $.body.dates[*] that is obviously saying that every item of the dates array must match a regexp. But pact:verify script doesn't respect it saying that
WARN: Ignoring unsupported matching rules {"match"=>"regex", "regex"=>"\\d{4}\\-\\d{2}\\-\\d{2}"} for path $['body']['dates'][*]

Then we tried to use a regexp in Pact.term with i modifier to make it case-insensitive, like
Pact.term(generate: "AA", matcher: /a+/i)
but we've got an error

  Value to generate "AA" does not match regular expression /a+\//

We like the ideas underlying Pact and it already does a lot. And we'd love to use it in our projects. But obviously, there is some room for improvements at least with specs integration and troubleshooting. We'd like to know if you are interested in collaboration, foreign pull requests to codebase and will you help us to investigate and solve the problems together.

@bethesque
Copy link
Member

Hi @frvade. The reason the Ruby impl doesn't support v3 yet is that this is open source work that I do in my free time, and there are only so many hours in the day. v3 has a significant increase in complexity compared to v2, and I just haven't worked up the mental energy to bite it off. One of the other reasons that v3 hasn't been implemented is that the longer term plan is to move to a shared Rust implementation (that already has v3 implemented) that the native languages would call using FFI bindings, so there didn't seem much point in implementing v3 in the Ruby.

Ruby is now an unpopular language, so finding people who want to contribute is hard. If you are interested, I would welcome your help with gratefulness!

In regards to the issues you have mentioned, can you please raise them as individual bugs so we can track them properly. They should all be able to be resolved. Personally, I don't use tear down blocks - I would recommend that your datasources are completely cleared at the start of each set_up block, as this is a much more reliable method of ensuring your state is correct, and allows you to inspect the remaining data after a test has finished to help identify issues.

@frvade
Copy link
Author

frvade commented Aug 10, 2018

Thanks for your quick response.

Is there any time estimations for the bindings implementations? 'Cause I'm not sure if we can help you with that but we surely can start fix the problems we encounter and implement some v3 features we need.

@frvade
Copy link
Author

frvade commented Aug 10, 2018

As for separate issues I will create them and we will try to provide pull requests at least for some of them.

@bethesque
Copy link
Member

There is no estimate on the v3 impl. I've found someone who may be interested in working on it however. We're discussing it now.

@bethesque
Copy link
Member

bethesque commented Aug 13, 2018

Also, I did some work on the v2 parsing in the pact-support gem and afterwards, I tried making an each like containing regular expressions - I couldn't recreate any issue, so I may have fixed it (I haven't had time to try recreating it with an older version). Can you try again with v1.7.2 of the pact-support gem before raising the issue please.

Please use this repository for recreating issues and providing an executable example of the problem: https://github.com/pact-foundation/pact-ruby-e2e-example

@bheemreddy181-zz
Copy link

bheemreddy181-zz commented Mar 25, 2019

@bethesque As we are explicitly telling what went wrong for the failures I don’t think we need data in the dB and most of the times when there is a failure for re-creating we run the test again to see what went wrong. My Expectation is we run these tests as part of CICD pipeline typically some where in travis-Ci or Circle-Ci and We can’t really look at the dB inserts which are created on those. Most of the times when we use database cleaner strategy we see the inserts in the logs and that’s more than enough for investigation what I feel when we recreate the issue and on Dev we can always switchoff database cleaner if we want and this should be something configurable.

@frvade agree ?

@bheemreddy181-zz
Copy link

I know this thread is old but @frvade did you get the above issues fixed by any chance ?

@bheemreddy181-zz
Copy link

Another Question comes here is @bethesque does CDC is intended to test the Syntax rather than going deeper to test the semantics as well. if just the syntax is what CDC is leaner towards then I can mock the underlying methods of REST-controller so that I don't have to populate the DB what I feel

@bethesque
Copy link
Member

As we are explicitly telling what went wrong for the failures I don’t think we need data in the dB and most of the times when there is a failure for re-creating we run the test again to see what went wrong. My Expectation is we run these tests as part of CICD pipeline typically some where in travis-Ci or Circle-Ci

Yes

and We can’t really look at the dB inserts which are created on those. Most of the times when we use database cleaner strategy we see the inserts in the logs and that’s more than enough for investigation what I feel when we recreate the issue and on Dev we can always switchoff database cleaner if we want and this should be something configurable.

Yes... is there a question there?

@bheemreddy181-zz
Copy link

bheemreddy181-zz commented Mar 27, 2019

Not really @bethesque I would like to see database-cleaner support so that I can clean up my DB post my test if you can add that it would be great. Where i can call the tear_down part from the setup.

@bethesque
Copy link
Member

does CDC is intended to test the Syntax rather than going deeper to test the semantics as well.

@bheemreddy181 I'm not sure what you define semantics as here. My gut feel is that semantics are important but behaviour is not.

For example. Imagine you are testing a POST to create an object. The contract test cares that the response contains the location of the newly created object in the headers, the response code is a 201, and perhaps the body contains the resource itself. The contract test does not care whether or not the object was actually created and stored in the provider (that's the job of the functional tests in the provider codebase). So, yes, you could stub out your database (or anything below the controller), if you also had "contract" tests to make sure that the layer you're stubbing out behaves the way you think it should.

@bethesque
Copy link
Member

@bheemreddy181 I use the database cleaner gems in all my Ruby pact tests. I wouldn't want to integrate that into pact - the concerns are completely different.

Here's an example in the Pact Broker project (it has a pact with the Pact Broker client project - meta...)

https://github.com/pact-foundation/pact_broker/blob/master/spec/service_consumers/pact_helper.rb#L10
https://github.com/pact-foundation/pact_broker/blob/master/spec/support/database_cleaner.rb

This is a bit more complex than a normal cleaner setup because I had to make it work with sqlite, mysql and postgres. The joys of open source development!

@bheemreddy181-zz
Copy link

Completely Agree with you @bethesque so in that case you feel like tear-down part is not yet all needed where you don't populate the DB itself? when I say semantics ( its the data getting created for that request in our DB )

@bethesque
Copy link
Member

I don't tear down after a spec, I tear down before a spec. Tearing down afterwards is dangerous, because you can't rely on the fact that a previous spec has correctly removed all the data (maybe it was interrupted, or failed, or there was some error cleaning). Cleaning before a spec means that you can be sure that the state you're dealing with is correct.

@bheemreddy181-zz
Copy link

Can you put an example of how to tear down before a pact spec on provider side?

@bethesque
Copy link
Member

Are we in ruby land where we're verifying a rack app, or are we verifying a running local application (where we set up the provider state by making an HTTP call to a 'provider state setup' endpoint)?

@bheemreddy181-zz
Copy link

we are in the land where we want to verify the response from HTTP Call for that app rather than verifying the rack app implementation and I am totally with you regards to that.

@bheemreddy181-zz
Copy link

@bheemreddy181-zz
Copy link

@bethesque is there a way I can say pact verify on provider side to use test_db than development dB ?

@bethesque
Copy link
Member

Can you put an example of how to tear down before a pact spec on provider side?

When I'm running migration specs in the Pact Broker (I'm actually testing that the migrations do what they are supposed to) I can't use the database cleaner because I'm doing DDL statements. I wrote my own cleaner that iterates through the tables and truncates (or drops) them in the right order.

The key is - the right order! I wrote this class to work out (dynamically) which order the tables and views could be dropped in:
https://github.com/pact-foundation/pact_broker/blob/4ee06460f10e8207ad904fa9fa6c4842e462ab59/tasks/database/table_dependency_calculator.rb

And here is the code that calls the tables in the right order and drops the records from each of them:
https://github.com/pact-foundation/pact_broker/blob/master/tasks/database.rb#L75

I would imagine running something like this at the start of each provider state set up call, if I was using a real database and a real app (I would just use the database-cleaner gem if I was verifying against a rack app).

@bheemreddy181-zz
Copy link

bheemreddy181-zz commented Mar 28, 2019

I am a bit confused here, why not a DatabaseCleaner in general to rollback the transactions, we generally use DatabaseCleaner for our rspec and functional tests(cucumber) which populates the db and cleans up once the transaction is done - which only does the rollback at transaction level why not the same way?

DatabaseCleaner.strategy = :transaction

Something like above where I can rollback the transaction

thats the reason why i was asking is there a way i can define the db for pact verify ? if i define the test db which i generally use for running my rspecs and cuckes same way i can rollback my transactions which would allow me not to write and additional cleanup.

@bheemreddy181-zz
Copy link

i got that this is what i am looking for : #167 (comment)

@bethesque
Copy link
Member

Are we all good here then?

@bheemreddy181-zz
Copy link

I have to verify the date matchers as you mentioned , help me understand one thing here pact-support gem will be sufficient for everything under pact or do we have to include other gems as well ?

@bethesque
Copy link
Member

You don't need to include any other gems.

@bheemreddy181-zz
Copy link

Not even the pact-broker?

@bheemreddy181-zz
Copy link

@bethesque I think this can be closed unless the owner wants to keep this open.

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

3 participants