Add URL tests that follow redirects and allow these to be run from the pipeline #3822
Conversation
|
The issue mentioned appears to be one in pytest core. I've raised pytest-dev/pytest#1351 but for the time being we can simply turn off the HTML report generation in Jenkins by removing the command line option. |
|
@davehunt looks like this needs a rebase |
| @@ -46,6 +47,8 @@ def url_test(url, location=None, status_code=requests.codes.moved_permanently, | |||
| :param req_kwargs: Extra arguments to pass to requests.get() | |||
| :param resp_headers: Dict of headers expected in the response. | |||
| :param query: Dict of expected query params in `location` URL. | |||
| :param allow_redirects: Boolean indicating whether redirects should be followed. | |||
There was a problem hiding this comment.
I would suggest follow_redirects instead. allow_redirects just seems confusing to me in a test suite mainly for redirects.
davehunt
force-pushed
the
redirect-endpoints
branch
from
5a7f7c9
to
91c2f7f
Compare
|
@alexgibson, @pmclanahan: Rebased and addressed review comments. |
davehunt
force-pushed
the
redirect-endpoints
branch
from
91c2f7f
to
6009365
Compare
|
Adding back a do-not-merge label as we need to sync some Jenkins config changes when this merges. |
davehunt
force-pushed
the
redirect-endpoints
branch
from
6009365
to
c4a4b82
Compare
davehunt
force-pushed
the
redirect-endpoints
branch
from
c4a4b82
to
3ff957e
Compare
|
@alexgibson @pmclanahan @jgmize Travis is happy now. Feel free to review, and we can sync up before this is merged next week to make sure I can do the necessary Jenkins updates. |
|
This looks good to me - Travis is now passing 😄, although I'll defer to either @pmclanahan or @jgmize in terms of technical review. Thanks @davehunt! |
|
r+ on the code, thanks @davehunt! Will merge once Jenkins changes have been made. |
| -e ALLOWED_HOSTS="*" \ | ||
| -e SECRET_KEY=foo \ | ||
| -e DEBUG=False \ | ||
| -e DATABASE_URL=sqlite:////tmp/temp.db \ |
There was a problem hiding this comment.
Just a note: if you don't need a DB that will stick around you can just set DATABASE_URL=sqlite:// which will cause it to use a purely in-memory database and never create a file.
|
r+ from me as well |
Add URL tests that follow redirects and allow these to be run from the pipeline
This patch introduces the tests from test_urls.py in mcom-tests. It also opens up the possibility of following all redirect tests to verify the final response code, however when I tested this I got a lot of 404s, most of these are probably expected due to random URLs used to verify redirects.
I've also renamed 'functional' to 'integration' in the Docker and Jenkins files and made it so that these can also run the 'redirect' tests. Note that
bedrock_functional_testsandbedrock_functional_tests_saucewill need to be modified (and likely renamed) as soon as this is merged. Let's coordinate when this is ready to land.I have noticed that due to a unicode character in a test name there's a problem generating the HTML report. This needs to be addressed before this can land, possibly by releasing a new version of pytest-html, but it shouldn't block getting a review.
@alexgibson, @pmclanahan, @jgmize: r?