internap / python-mockserver-friendly-client

Friendly python client to James D. Bloom's awesome MockServer
Apache License 2.0
14 stars 12 forks source link

Add verify method now we can verify single request #8

Closed rocketb closed 5 years ago

rocketb commented 6 years ago

With this feature, we can verify single request without mocking. It will be helpful in some proxy cases.

Something like that:

from mockserver import MockClient, request, times

client = MockClient('localhost:1080')
client.verify_request(request(method='GET', path='/path', querystring={'test': 'test'}), timing=times(1))
rocketb commented 6 years ago

So with your agreement, I chose path 3 :sweat_smile:. In case that we don't check stubing and verifying works for hits of MockServer, we can just hit the mock server with any request, so we don't need .stub in setUp step. See Mock server Verifications

I move some expectations checks in verify test class, because now .verify_expectations just iterate throw all items in expectations dict, please check it out.

rocketb commented 6 years ago

Ok, you're right. That's one little update, one step closer to the goal. )

lindycoder commented 6 years ago

There's a wierd flakyness on this test

FAIL: test_verify_all_expectations (test.test_basic_verifying.TestBasicVerifying)

It failed the current check on 2.7 and 3.4 and before your last change it also failed on 2.7

does it pass locally for you?

lindycoder commented 6 years ago

it's as if the mock-server was not cleaned properly between 2 tests... as there's a stub on /path that lingers?

everything is fine when i run that locally... so wierd

rocketb commented 6 years ago

Yep, locally it is fine... I confirm somehow mock server doesn't clean up on CI

lindycoder commented 6 years ago

all right i made another PR based on yours and did some experimentation and here's my result:

add

def tearDown(self):
        self.client.reset()

in test/init.py, it seems to resolve most test pollution issues.

After that there's only this test that is very very flaky: test_verify_request_received_once (test.test_basic_verifying.TestBasicVerifying) saying AssertionError: Request not found exactly once, expected:<{ }> but was:<>

as if the request never happened, so it looks a lot like race condition

Not sure how to address this:

Do you have an opinion on this matter?

Marx314 commented 6 years ago

or within the setup of MockServerClientTestCase but after the reset

add a call to retrieve?type=RECORDED_EXPECTATIONS until it's empty?

-> http://www.mock-server.com/proxy/record_and_replay.html

lindycoder commented 6 years ago

@Marx314 Nice to see you!

doing that could solve the problem of having to put a reset in the teardown, but that won't help the tests that does request.get directly followed by verifythat whines it doesn't have received a request yet.

rocketb commented 6 years ago

Since this is network or MockServer issue but not the bug of client lib, the best way on my opinion will be using mocks in future. For now, I suggest adding call of retrieve in setUp and the pooling of .verify method.

lindycoder commented 6 years ago

All right let's try stuff,

could you cherry pick or rebase onto #11 that might give us some more insight on the problem.

(And, somewhat annoying, i haven't been able to reproduce the flakyness in --verbose mode... :P)

lindycoder commented 6 years ago

interesting... on that last failure:

the test does

but the logs from the container sees:

and that extra reset is the setup of the next test. So somehow the next test starts before that one finished!?

lindycoder commented 6 years ago

All right i got it working!

Check #15 that will stabilize tests a lot.

I created #14 with your first 3 commits and a little addition, stubbing the first verify test and i wasn't able to make it fail yet.

So you can either rebase off #15, squash your first 3 commit along with the stubbing i added in #14

Or we can merge #14, what do you think?

If you want the retrieve functionality we can make a new PR but i'd rethink the api a bit.

Thanks for your contribution, sorry that i made it complicated to make reliable tests, i think testing the container is worth it :)

lindycoder commented 5 years ago

Have been replaced by #14