IMPORTANT: Add API Testing

[jacob-macleod]

At the moment, the API is not tested. This should be fixed by adding extensive unit tests. This is essential, and very important, to ensure the code works long term with no errors. So far, it has only been slightly tested manually, and so there is the potential for significant issues in the code. The pipeline should run unit tests which are in the the testing folder, if they are named following a pattern which pytest can detect

[jaymeklein]

I would love to help with this issue and this project. I have some questions about this issue:

• All endpoints must have testing?
• How many different tests each endpoint must have?

Also, you can assing this issue to me if you want to.

[jacob-macleod]

Great, thanks! Yes, all the endpoints need to be tested, except /api/firebase-config in backend/routes/api/authentication.py - this API route is no longer used and will be removed in the future. In the main branch, goals.py isn't a web route - it just contains functions. In the dashboard-page branch, the functions in goals.py are turned into API routes, but that'll only be ready when the branch is merged into main. So all the API routes in the following files need to be tested:

• backend/routes/api/authentication.py
• backend/routes/api/card_management.py
• statistics.py These are the only files containing API routes.

In terms of the endpoint tests, one part would be checking that the API accepts or rejects the right sort of data. But the priority would be that the API does what is expected when you pass it valid or extreme data. For example, when you go to the api/create-account route, is an account created? Can you pass it any data which makes it behave differently than expected? Also, a priority would be to see what happens in different situations - for example, what happens when you try to get information from a user who doesn't exist?

A guide to how the code works, and for testing the code, is at CONTRIBUTING.md. You'll need to cloning the repo, and testing the local installation. Then you can set it to save to local .json files rather than Firebase, which are easier to check if they have the right data. Then, the unit tests may be able to read these files. Also, you are free to change any classes to make it easier to test, so long as the code won't dramatically change - in particular, the database classes are relevant.

Feel free to ask me any questions about this, including about the CONTRIBUTING.md guide - hopefully its easy to follow

Also, make sure that you write the code so that if you clone the repository, and run pytest testing/, assuming requirements are installed, the tests will run. For example, if your tests rely on the code being setup so that it writes to a local file, the test should do this configuration first (in other words, changing the file that is used to determine whether the database is firebase or local). This is to replicate the process that the automatic pipeline that runs tests will make.

[jaymeklein]

Thanks for assigning me to this issue! As soon as i get home, i plan to start to work on it.

[jaymeklein]

Can i use any testing framework like unittest, or should i use just assert for this?

[jacob-macleod]

You can use any testing framework you like, so long as you add the packet to requirements.txt (The pipeline, which automatically runs the tests, installs the packages in requirements.txt before running pytest)

[jaymeklein]

Hey, @jacob-macleod. I'm wondering, how do you want these tests to be?

One method for each endpoint, with many assertions, like this:

    def test_create_account(self) -> None:
        valid_dummy = {
            "userID": "1",
            "displayName": "Dummy"
        }

        response = self.post_api(Routes.ROUTE_CREATE_ACCOUNT['url'], valid_dummy)
        response_json = response[0]
        self.assertTrue(expr=response_json['success'],
                        msg=f'Invalid return for "create_account" method. '
                            f'Should be True but was {response_json['success']}')

        error_dummy = {
            "userID": "",
            "displayName": ""
        }

        response = self.post_api(Routes.ROUTE_CREATE_ACCOUNT['url'], error_dummy)
        response_json = response[0]
        self.assertFalse(expr=response_json['success'],
                         msg='Invalid return for "create_account" method, should not create a nameless user.')

Or should I split each assertion on different methods?

    def test_create_valid_account(self) -> None:
        valid_dummy = {
            "userID": "1",
            "displayName": "Dummy"
        }

        response = self.post_api(Routes.ROUTE_CREATE_ACCOUNT['url'], valid_dummy)
        response_json = response[0]
        self.assertTrue(expr=response_json['success'],
                        msg=f'Invalid return for "create_account" method. '
                            f'Should be True but was {response_json['success']}')

    def test_create_invalid_account(self) -> None:
        error_dummy = {
            "userID": "",
            "displayName": ""
        }

        response = self.post_api(Routes.ROUTE_CREATE_ACCOUNT['url'], error_dummy)
        response_json = response[0]
        self.assertFalse(expr=response_json['success'],
                         msg='Invalid return for "create_account" method, should not create a nameless user.')

The second way is verbose and specific, the first one will stop on the first AssertionError on the method.

Particularly, i think the second way is more organized, easier to understand and to find the problem:

[jacob-macleod]

Yeah, the second one would probably make the most sense

[jaymeklein]

Perfect, the backend-testing branch has already some unitary tests. Here is the branch i'm currently working on, the file test_api.py has the most changes, and the unittests come just after test_check_request_json method.

test_api.py

If you want any specific test, just let me know. 👍

[jacob-macleod]

Looks good, thanks! Just one point, the serve_credentials function doesn't need to be tested, because its not being used anymore and will be removed in the future.

I see by the function for that, you have some comments relating to setting up a test firebase instance. Because the tests are run automatically in a pipeline, the instance would have to be integrated into the pipeline. There are two possible ways - either having a pipeline stage where test instance credentials are saved from github secrets, like the real credentials in the current pipeline, or by deploying some sort of testing deployment of the code connected to firebase, and testing that. Of course, that's outside the main scope of this issue, so you don't have to explore that - it would be a secondary thing, possibly needing a separate issue.

[jaymeklein]

The comment you saw about a firebase instance was exactly about the serve_credentials function, i forgot that you said that it does not need to be tested.

Besides this function, do you think i'm gonna need a Firebase instace on my local development environment?

I commented that because there was no other function that had no local database connection.

I'm wondering if it's possible to test all the endpoints using just the local database config.

[jacob-macleod]

No - other than that, all the other endpoints just need a local database

[jacob-macleod]

@jaymeklein is this issue completed now - am I free to close it?

[jaymeklein]

Yes, i think you're free to close this issue. Please let me know if there's something missing on the tests. 👍