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

pytest_httpserver: extend HTTPServer to support writing behave test s… #166

Merged
merged 3 commits into from
Sep 2, 2022

Conversation

matez0
Copy link
Contributor

@matez0 matez0 commented Aug 28, 2022

…teps in real order

HTTPServer is suitable in itself to use in behave tests,
but because of the expectation on a request has to be done
before the request is performed
the test steps of checking the request and sending the response
also has to be before the test step triggering the request.
This makes a confusing test specification like:

Then SUT sends an other request to server
When server responds something
When a request is sent to SUT
Then SUT responds something

Using the BlockingHttpServer, the server blocks until the request is asserted
and then it blocks again until the response is performed.
The test steps can be written in real order:

When a request is sent to SUT
Then SUT sends an other request to server
When server responds something
Then SUT responds something

@codecov
Copy link

codecov bot commented Aug 28, 2022

Codecov Report

Merging #166 (8aa364b) into master (e75910c) will increase coverage by 0.34%.
The diff coverage is 95.94%.

❗ Current head 8aa364b differs from pull request most recent head 642c70f. Consider uploading reports for the commit 642c70f to get more accurate results

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
+ Coverage   94.54%   94.89%   +0.34%     
==========================================
  Files           3        4       +1     
  Lines         477      548      +71     
==========================================
+ Hits          451      520      +69     
- Misses         26       28       +2     
Impacted Files Coverage Δ
pytest_httpserver/httpserver.py 95.15% <93.40%> (-0.32%) ⬇️
pytest_httpserver/__init__.py 100.00% <100.00%> (ø)
pytest_httpserver/blocking_http_server.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@matez0 matez0 force-pushed the extension-for-behave branch 3 times, most recently from 29a012a to 95e903a Compare August 28, 2022 16:28
@matez0
Copy link
Contributor Author

matez0 commented Aug 28, 2022

I need help to figure out what is the problem with the coding style.
For me, flake8 and pycodestyle do not show anything.

@csernazs
Copy link
Owner

I've started looking at your code.

Regarding coding style errors: since you started working on this, a pre-commit check has been added to the project, so you can set it up if you want so it will check all the coding style + linting for each commit. To do that you would need to run pre-commit install which will set up the hooks.

If you don't want to do that (that's also fine, but the CI will catch the errors), you can run make precommit to fix the issues. It seems that some imports are not sorted correctly, but if you run this it will fix the issues.

pytest_httpserver/blocking_http_server.py Outdated Show resolved Hide resolved
tests/test_blocking_http_server.py Outdated Show resolved Hide resolved
tests/test_blocking_http_server.py Outdated Show resolved Hide resolved
pytest_httpserver/blocking_http_server.py Outdated Show resolved Hide resolved
@matez0 matez0 force-pushed the extension-for-behave branch 5 times, most recently from 8aa364b to fcd95f5 Compare September 1, 2022 00:25
@matez0 matez0 requested a review from csernazs September 1, 2022 00:32
@csernazs
Copy link
Owner

csernazs commented Sep 1, 2022

@matez0 I looked at your changes briefly and it looks good for the first sight. I think I'll review it today evening. Thanks!

The functionalities of the interface are responding:
- with a JSON object,
- with raw data,
- with a Response object.

This separation makes these functionalities reusable by derivation.
…test steps in real order

Although HTTPServer is suitable in itself to use in behave tests,
but because of the expectation on a request has to be done
before the request is performed
the test steps of checking the request and sending the response
also has to be before the test step triggering the request.
This makes a confusing test specification like:

Then SUT sends an other request to server
When server responds something
When a request is sent to SUT
Then SUT responds something

Using the BlockingHttpServer, the server blocks until the request is asserted
and then it blocks again until the response is performed.
The test steps can be written in real order:

When a request is sent to SUT
Then SUT sends an other request to server
When server responds something
Then SUT responds something
@matez0
Copy link
Contributor Author

matez0 commented Sep 1, 2022

Thank you!
I removed the _set_response which was actually the respond_with_response.
This way, the name and the goal of the method is more clear and the protected abstract method was not nice neither.
The two other responding methods of the base class are now reducible to that method.

Copy link
Owner

@csernazs csernazs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much for the refactoring I think it looks very good and nicely separated everything!

I have found minor things, so same applies as before: if you don't want to do that because you already put the effort, please let me know, and I'll do it.

Documentation also needs to be fixed and updated, but I'll do that once your PR is merged. I need to prepare a release note also.

pytest_httpserver/blocking_http_server.py Show resolved Hide resolved
pytest_httpserver/blocking_http_server.py Show resolved Hide resolved
pytest_httpserver/httpserver.py Show resolved Hide resolved
pytest_httpserver/httpserver.py Show resolved Hide resolved
@matez0
Copy link
Contributor Author

matez0 commented Sep 2, 2022

If you can fixup this changes quickly, that would be fine for me.
I may be able to do it in the next half day or surely 10 days later, because of having no computer soon. :)

@csernazs
Copy link
Owner

csernazs commented Sep 2, 2022

Ok, then I merge this, and will fix in another PR.
Thanks very much again!

@csernazs csernazs merged commit c0a581f into csernazs:master Sep 2, 2022
@csernazs csernazs mentioned this pull request Sep 3, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants