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

Add Tests for ICAT Backend #191

Conversation

MRichards99
Copy link
Collaborator

@MRichards99 MRichards99 commented Dec 8, 2020

This PR will close #150

Description

This branch focuses on adding tests for the ICAT backend, but I've also added tests for non-backend specific components, as well as rewriting the existing unittest tests (for the DB backend) to pytest and expanding the coverage of testing that backend to cover the core aspects of that backend. The test coverage on the DB backend isn't as extensive as the ICAT backend because in theory, the DB backend shouldn't be used in production again. The coverage has been expanded however, so I feel this is a good middle ground for the time being.

In reasonable chronological order, here's a list of tests added in this branch:

  • DateHandler testing
  • Config testing
  • All filters for ICAT backend (placed in test/icat/filters/, 1 file per filter)
  • Test that each backend can be created successfully
  • FilterOrderHandler testing (using ICAT filters)
  • Tests for ICATQuery class
  • Tests (valid and invalid) for all request types on ICAT backend (including ISIS endpoints), using the investigation entity as test
  • Test to check all endpoints exist (particularly to ensure all entity endpoints exist) and that they have the correct HTTP methods (e.g. GET, DELETE, POST)
  • Session handling for ICAT backend
  • Convert all existing tests to pytest (I split these up into a file per test class, instead of one big test_helpers.py file)
  • Tests for GET requests on DB backend
  • Tests for ISIS endpoints on DB backend

To add all these tests, there are a number of API changes that I've had to make (listed in rough chronological order):

  • The contents of backends.py is now inside a function, which is parsed a string value to create an instance of a backend
  • Added a Nox session to execute the tests (documentation of this at the end of README.md one of the final commits on this branch
  • The Config class can have a path parsed to it that (defaults to config.json) - used for testing this class and providing a dummy config
  • Add pytest-cov to show test coverage
  • Added test credentials to use in the config
  • Sort the input of flatten_query_included_fields() to ensure the same output order - makes it testable
  • Added pytest-icdiff so side-by-side diffs are shown on test assertion failures when -vv is used
  • Refactor main.py so a backend specific test isn't tied to the backend value in the config (explained in depth below).
  • Move QueryFilterFactory to its own file (it was in database/helpers.py before even though it's a non-backend specific class)

I spent a lot of time decoupling the backend defined in config.json from the tests. This is important because if an ICAT backend test runs on a Flask test client set up with the DB backend, it will fail. In a perfect world, a single test should be able to used for both backends, but the slight differences between the two backends means I didn't go down this path. I did try it when I expanded the DB backend tests, but I found it reduced the readability of the test function I was experimenting with, and things got complicated very quickly so I chose to abandon that idea. A separate test directory per backend means any future changes with the ICAT backend won't affect tests for the DB backend.

This starts with the refactor of main.py which separates the existing code in there into multiple functions. The main difference the functions make is that a different Flask app object (different to the app variable created in main.py) can be used to set up the API (used for testing). The start of create_api_endpoints() sets the backend type on the config object if the Flask app has a TEST_BACKEND config option set (this is set in the Flask test apps found in conftest.py). The backend is then created and parsed to each type of Flask resource in the API. This is different to what happened before (the backend was created inside each endpoint file). The backend must be parsed (as opposed to created at the top of a file) because the TEST_BACKEND config option would never be set before the backend was created (code not in a function in a file that's imported is executed straight away). This is the main factor behind all these changes, and preventing circular imports limited my options on this further. To accommodate the backend being parsed, the classes for the session endpoint and the ISIS endpoints are put inside a function (in exactly the same way this is done for each entity endpoint).

Testing instructions

  • Review the tests I've added
  • Check all tests pass
  • Review the changes I've made to the API
  • Review changes to test coverage

Agile board tracking

connect to #150

…ructure

- Just a few comments to help me work out what to do with the existing tests for DB backend. These will be moved to work with Pytest once the new test structure has been defined
- Currently getting a 403 on these changes but I'm not sure the code is at fault...
- This also updates a couple of dependencies as marked by safety, which didn't get merged in from a recent git merge
- This will run the discovered unit tests in multiple versions of Python (unless nox is specified with -p [version_num] which is probably what I'll do for the majority of the time, just useful to test multi-version compatability)
- This will make testing the configuration easier since you cannot guarantee what the contents of config.json will be
- Default is what it was before
- nox -s tests -- --cov is a good starting point for usage
- I'm not sure how much I trust the output - there's around half coverage for both DB & ICAT backend's helper files, even though I've currently got the ICAT backend configured...
- This commit also adds getters in config.py
- This will make it more convenient to use through the tests in the repo
- Since they're placed in conftest.py, pytest will automatically pick these up, they don't need to be imported into the places I use them in
- That function would return the elements of a list in different order each time (despite identical inputs each time) so this could prove more difficult to test
…CATQuery

- This fixture also removes the data from ICAT at the end of the test
- This commit also includes a skeleton for the remaining tests for this class
- This adds validation on skip and limit values on the respective filters
@MRichards99 MRichards99 requested a review from VKTB December 8, 2020 14:52
@MRichards99 MRichards99 linked an issue Dec 8, 2020 that may be closed by this pull request
Copy link
Contributor

@VKTB VKTB left a comment

Choose a reason for hiding this comment

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

Good work overall, just a few things that need looking at.

datagateway_api/common/backends.py Outdated Show resolved Hide resolved
datagateway_api/common/backends.py Outdated Show resolved Hide resolved
datagateway_api/common/config.py Outdated Show resolved Hide resolved
test/db/endpoints/test_get_by_id_db.py Outdated Show resolved Hide resolved
test/db/endpoints/test_get_with_filters.py Outdated Show resolved Hide resolved
test/icat/test_query.py Outdated Show resolved Hide resolved
test/icat/test_session_handling.py Show resolved Hide resolved
test/icat/test_session_handling.py Show resolved Hide resolved
test/icat/test_session_handling.py Outdated Show resolved Hide resolved
test/test_base.py Outdated Show resolved Hide resolved
MRichards99 and others added 5 commits January 4, 2021 10:45
- This has been removed in a different branch so I've removed the documented details about it in this branch and adjusted it according to the new solution found
Co-authored-by: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com>
…facilities/datagateway-api into feature/test-multiple-backends-#150
@MRichards99 MRichards99 requested a review from VKTB January 4, 2021 13:42
An error occurred while trying to automatically change base from feature/fix-code-linting-#184 to feature/add-code-linting-#165 January 4, 2021 15:54
An error occurred while trying to automatically change base from feature/fix-code-linting-#184 to feature/add-code-linting-#165 January 4, 2021 15:54
An error occurred while trying to automatically change base from feature/fix-code-linting-#184 to feature/add-code-linting-#165 January 4, 2021 15:54
An error occurred while trying to automatically change base from feature/fix-code-linting-#184 to feature/add-code-linting-#165 January 5, 2021 09:36
@MRichards99 MRichards99 changed the base branch from feature/fix-code-linting-#184 to feature/remove-sql-dependency-from-backends-#154 January 5, 2021 09:38
MRichards99 and others added 3 commits January 5, 2021 10:20
- This commit also adds the "tests" session to the list of Nox sessions
…documentation-#190

Add ICAT Backend Documentation
Copy link
Contributor

@VKTB VKTB left a comment

Choose a reason for hiding this comment

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

Only tried running the ICAT backend tests, which successfully passed, as I don't have a local ICAT instance and therefore can't configure the config with a valid DB_URL

@MRichards99
Copy link
Collaborator Author

As confirmation for Viktor, all the tests (DB, ICAT, non-backend specific) pass as shown below.

[root@localhost datagateway-api]# nox -p 3.6 -s tests
nox > Running session tests-3.6
nox > Re-using existing virtual environment at .nox/tests-3-6.
nox > poetry install
Installing dependencies from lock file

No dependencies to install or update

Installing the current project: datagateway-api (0.1.0)
nox > pytest 
=================================================== test session starts ====================================================
platform linux -- Python 3.6.8, pytest-6.1.2, py-1.9.0, pluggy-0.13.1
rootdir: /root/seg-dev/datagateway-api
plugins: cov-2.10.1, icdiff-0.5
collected 217 items                                                                                                        

test/test_backends.py ...                                                                                            [  1%]
test/test_config.py .........................                                                                        [ 12%]
test/test_date_handler.py ............                                                                               [ 18%]
test/test_endpoint_rules.py .........                                                                                [ 22%]
test/test_get_filters_from_query.py ........                                                                         [ 26%]
test/test_get_session_id_from_auth_header.py ...                                                                     [ 27%]
test/test_is_valid_json.py .........                                                                                 [ 31%]
test/test_queries_records.py ......                                                                                  [ 34%]
test/db/test_entity_helper.py .....                                                                                  [ 36%]
test/db/test_query_filter_factory.py ..............                                                                  [ 43%]
test/db/test_requires_session_id.py ....                                                                             [ 45%]
test/db/endpoints/test_count_with_filters_db.py ..                                                                   [ 46%]
test/db/endpoints/test_findone_db.py ..                                                                              [ 47%]
test/db/endpoints/test_get_by_id_db.py ..                                                                            [ 47%]
test/db/endpoints/test_get_with_filters.py ....                                                                      [ 49%]
test/db/endpoints/test_table_endpoints_db.py ........                                                                [ 53%]
test/icat/test_filter_order_handler.py ......                                                                        [ 56%]
test/icat/test_query.py ......                                                                                       [ 58%]
test/icat/test_session_handling.py .......                                                                           [ 62%]
test/icat/endpoints/test_count_with_filters_icat.py ..                                                               [ 63%]
test/icat/endpoints/test_create_icat.py ....                                                                         [ 64%]
test/icat/endpoints/test_delete_by_id_icat.py ..                                                                     [ 65%]
test/icat/endpoints/test_findone_icat.py ..                                                                          [ 66%]
test/icat/endpoints/test_get_by_id_icat.py ..                                                                        [ 67%]
test/icat/endpoints/test_get_with_filters_icat.py ....                                                               [ 69%]
test/icat/endpoints/test_table_endpoints_icat.py ........                                                            [ 73%]
test/icat/endpoints/test_update_by_id_icat.py ..                                                                     [ 74%]
test/icat/endpoints/test_update_multiple_icat.py .....                                                               [ 76%]
test/icat/filters/test_distinct_filter.py .......                                                                    [ 79%]
test/icat/filters/test_include_filter.py ............                                                                [ 85%]
test/icat/filters/test_limit_filter.py .........                                                                     [ 89%]
test/icat/filters/test_order_filter.py .....                                                                         [ 91%]
test/icat/filters/test_skip_filter.py ....                                                                           [ 93%]
test/icat/filters/test_where_filter.py ..............                                                                [100%]

===================================================== warnings summary =====================================================
.nox/tests-3-6/lib/python3.6/site-packages/urllib3/connectionpool.py:988
test/test_config.py::TestGetICATProperties::test_valid_icat_properties
test/icat/filters/test_skip_filter.py::TestICATSkipFilter::test_valid_skip_value[typical]
test/icat/filters/test_skip_filter.py::TestICATSkipFilter::test_valid_skip_value[boundary]
  /root/seg-dev/datagateway-api/.nox/tests-3-6/lib/python3.6/site-packages/urllib3/connectionpool.py:988: InsecureRequestWarning: Unverified HTTPS request is being made to host 'PREPROD'. Adding certificate verification is strongly advised. See: https://urllib3.readthedocs.io/en/latest/advanced-usage.html#ssl-warnings
    InsecureRequestWarning,

-- Docs: https://docs.pytest.org/en/stable/warnings.html
======================================= 217 passed, 4 warnings in 570.30s (0:09:30) ========================================
nox > Session tests-3.6 was successful.
[root@localhost datagateway-api]# git status
On branch feature/test-multiple-backends-#150
Your branch is up to date with 'origin/feature/test-multiple-backends-#150'.

nothing to commit, working tree clean
[root@localhost datagateway-api]# date
Tue Jan  5 14:36:16 UTC 2021

@MRichards99 MRichards99 merged commit b11001d into feature/remove-sql-dependency-from-backends-#154 Jan 5, 2021
@MRichards99 MRichards99 deleted the feature/test-multiple-backends-#150 branch January 5, 2021 14:40
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.

Test multiple backend types within unit tests
2 participants