Skip to content

Testlib request methods#5347

Merged
jburel merged 29 commits intoome:developfrom
will-moore:testlib_request_methods
Aug 10, 2017
Merged

Testlib request methods#5347
jburel merged 29 commits intoome:developfrom
will-moore:testlib_request_methods

Conversation

@will-moore
Copy link
Copy Markdown
Member

@will-moore will-moore commented Jun 27, 2017

What this PR does

Cleanup of omeroweb.testlib Django client request methods, based on https://trello.com/c/flvJ728g/31-web-api-tests-to-include-csrf.

Previously we've had a number of testlib methods such as:

_post_response
_post_response_json
_csrf_post_response
_csrf_post_response_json
_csrf_post_json
_csrf_put_json
_delete_response
_csrf_delete_response
_csrf_delete_response_json
_get_response
_csrf_get_response
_get_response_json

These try to cover a matrix of:

  • GET, POST, PUT, DELETE
  • With or with CSRF tokens
  • POST data as query string or JSON content
  • Handling response as text or JSON

These are replaced by a smaller number of methods.
For all POST, PUT & DELETE we first test without CSRF (check for 403 response) then with CSRF.

(This means that all the test_api_* tests are now being tested for CSRF checks, as per trello card above).

get: GET without CSRF (parameter for CSRF option)
get_json: as above, but handles JSON response
post: POST - data encoded in query string
post_json: POST, encodes data as JSON and handles JSON response
put_json: PUT, encodes data as JSON and handles JSON response
delete_json: DELETE, encodes data as JSON and handles JSON response

csrf_response: underlying helper method is available if you want to do POST/PUT/DELETE without doing the CSRF test.

All ```Omeroweb/test/integration``` tests have been updated to use new methods.
Original methods above are marked with deprecation warnings.

Testing this PR

  1. Tests should be green.


:param django_client: Django test Client
:param request_url: The url to request
:param data: A dict of data to include as json content
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

missing method argument

def _post_response(django_client, request_url, data, status_code=403,
content_type=MULTIPART_CONTENT, **extra):
warnings.warn(
"This method is deprecated as of OMERO 5.4.0", DeprecationWarning)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe indicate the one to use in the message

@jburel jburel added the develop label Jun 27, 2017
@mtbc
Copy link
Copy Markdown
Member

mtbc commented Jun 29, 2017

Would you mind temporarily excluding this so we can try to get #5349 in?

@will-moore
Copy link
Copy Markdown
Member Author

@mtbc Added exclude label to description.

@jburel
Copy link
Copy Markdown
Member

jburel commented Jul 31, 2017

@will-moore could you remove the exclude label and fix the merge conflict
now that #5349 is in

@will-moore
Copy link
Copy Markdown
Member Author

Travis failure: "This job ran on our Trusty environment, which is gradually becoming our default Linux environment. You can add dist: precise in your .travis.yml file to continue using Precise."

@jburel
Copy link
Copy Markdown
Member

jburel commented Aug 1, 2017

@will-moore could you roll back your latest commit?
I am adjusting the travis file in gh-5409
so we can update pytables (see gh-5407)

@will-moore will-moore force-pushed the testlib_request_methods branch from 02a3740 to cea6fa3 Compare August 2, 2017 06:34
@snoopycrimecop snoopycrimecop mentioned this pull request Aug 10, 2017
2 tasks
@jburel
Copy link
Copy Markdown
Member

jburel commented Aug 10, 2017

Thanks for updating message
Tests are passing
Merging so we can unlock other PRs

@jburel jburel merged commit e80eba5 into ome:develop Aug 10, 2017
@sbesson sbesson added this to the 5.4.0 milestone Oct 6, 2017
@will-moore will-moore deleted the testlib_request_methods branch February 18, 2019 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants