Add automated tests for the categorization insertion form

[ZhongweiL]

As mentioned by @khatchad in https://github.com/ponder-lab/Imperative-DL-Study-Web-App/issues/86#issuecomment-821187291_

[mZneit]

@ZhongweiL, can you please open a new pull request with the changes made to tests.py file? And please checkout this section on running tests https://docs.djangoproject.com/en/3.1/topics/testing/overview/#running-tests

[ZhongweiL]

@mZneit I'm not completed with the tests, all I have is a test that asserts on some dummy values, should I upload that anyway?

[mZneit]

yes no problem.

[mZneit]

Running the tests will create a test database that we as users have no permission to create, that's why we were denied access. According to the django doc: The test database is created by the user specified by USER (in the settings.py file), so you’ll need to make sure that the given user account has sufficient privileges to create a new database on the system.

[ZhongweiL]

Should we grant the specified user that privilege? Or create another user with that privilege and modify the USER section in settings.py to that new user?

[mZneit]

It would be easier to grant privilege, but it wouldn't be necessary if we can run the sql_dump_file.sql and connect to the database locally in which case we are able to create the test database. I already did that and the test database was created but the problem is that the models were not migrated. This is due to some parameter called managed that was set to False in all the models. @ZhongweiL if you can follow what I did and try to apply migrations to the test database then we can run the tests.

[khatchad]

This is the check constraint for he categorizations table.

[khatchad]

Note also the non nulls.

[ZhongweiL]

@mZneit It worked, thanks!

[mZneit]

Great. You didn't set the parameter managed = True, right? We want it fixed using the command line arguments.

[ZhongweiL]

I did set them to true to make it work

[mZneit]

Ok, it would be better to focus on writing the test cases for now and if some errors related to unmanaged models were raised, we'll try to solve them later.

[ZhongweiL]

I am trying to access the new category that I just created, but I am getting index out of range error which I think is because the categorization is not created successfully in the database, is there any way I can know why the categorization is not created successfully by the post request?

from django.test import TestCase
from ponder.models import Categorization

test_cases = [{"problem_category": "test" }]

class AddCategorizationFormTests(TestCase):
    def setUp(self):
        self.data = test_cases[0]
        self.client.post("/ponder/categorizations/add?commit=00a33f7e060caa6616c15003bdac95b7926b76ae", data=self.data)
        #list out of range
        self.newCategory = Categorization.objects.filter(sha='00a33f7e060caa6616c15003bdac95b7926b76ae')[0]

    def test_sha_not_null(self):
        print('hello')

[mZneit]

You need to update the sql_dump_file.sql every time you change the database. Instructions on how to update it are found in the readme file.

[ZhongweiL]

Where in sql_dump_file.sql should I be looking at? I'm thinking commenting out all the constraints in the tables.

[mZneit]

@ZhongweiL I think you need to go through this page to write tests. https://docs.djangoproject.com/en/3.2/topics/testing/tools/

[ZhongweiL]

I figured out how to authenticate myself in the test setup. I'm able to make requests to most routes, but I am getting an error accessing the form page. I also tried to access the form page from the web client with the local database, but I wasn't able to do that either and I got a different error message.

[mZneit]

@ZhongweiL can you examine the test database and check if you can see the tables? (Categorizations, Commits, Problem Categories tables, etc...)

[ZhongweiL]

@mZneit Yes, they are all in the test database.

[mZneit]

Are you still setting managed=true in the models?

[ZhongweiL]

Yes

[mZneit]

@ZhongweiL I updated the readme file on how to run the tests. I added a new file that you have to configure with your info to run the tests. Please let me know if things didn't work.

[ZhongweiL]

@mZneit I'm still getting the same error

[mZneit]

I modified the tests.py file yesterday, you need to include the new changes on the top of the tests.py file before importing any model or form.

[ZhongweiL]

I did include them

[mZneit]

Also you need to set managed to False again. I suggest that you clone the repo just in case there were some changes that shouldn't be applied.

[khatchad]

https://github.com/ponder-lab/Imperative-DL-Study-Web-App/blob/a9e4c1947ce4e43b638b44e133c08bb62bc872d0/ponder/tests.py#L50-L51

This doesn't look like it's testing the form at all. It's just testing whether the test data is populated in the field.

[khatchad]

https://github.com/ponder-lab/Imperative-DL-Study-Web-App/blob/a9e4c1947ce4e43b638b44e133c08bb62bc872d0/ponder/tests.py#L59

What does this do? We need many comments here.

[khatchad]

We need a lot of comments here too: https://github.com/ponder-lab/Imperative-DL-Study-Web-App/blob/a9e4c1947ce4e43b638b44e133c08bb62bc872d0/ponder/tests.py#L66-L79

I cannot tell what is being tested here.

[khatchad]

@ZhongweiL please also setup GitHub actions to run these tests using CI. They should run upon every new commit.

[khatchad]

Also, since a of the test data is blank and that there are guards everywhere only testing is the length of the data is greater than 0, many fields are not being tested.

[khatchad]

Lastly, the test suite should fail since we have current problems with the form, e.g., #103, #98.

[ZhongweiL]

https://github.com/ponder-lab/Imperative-DL-Study-Web-App/blob/a9e4c1947ce4e43b638b44e133c08bb62bc872d0/ponder/tests.py#L59

What does this do? We need many comments here.

It basically checks if all non-empty fields in the test case are being recorded in the form. So if a field is not empty in the test case, the field should exist in the form and it shouldn't be empty.

[ZhongweiL]

We need a lot of comments here too: https://github.com/ponder-lab/Imperative-DL-Study-Web-App/blob/a9e4c1947ce4e43b638b44e133c08bb62bc872d0/ponder/tests.py#L66-L79

I cannot tell what is being tested here.

This one is for the check constraints in the database that you showed me earlier. The form shouldn't be valid if the data do not meet these conditions.

[khatchad]

We need a lot of comments here too: https://github.com/ponder-lab/Imperative-DL-Study-Web-App/blob/a9e4c1947ce4e43b638b44e133c08bb62bc872d0/ponder/tests.py#L66-L79

I cannot tell what is being tested here.

This one is for the check constraints in the database that you showed me earlier. The form shouldn't be valid if the data do not meet these conditions.

Sounds like an ideal place for comment.

Also, the name of this test is not informative. Are you testing a valid case or an invalid case? What test case is this?

[ZhongweiL]

We need a lot of comments here too: https://github.com/ponder-lab/Imperative-DL-Study-Web-App/blob/a9e4c1947ce4e43b638b44e133c08bb62bc872d0/ponder/tests.py#L66-L79

I cannot tell what is being tested here.

This one is for the check constraints in the database that you showed me earlier. The form shouldn't be valid if the data do not meet these conditions.

Sounds like an ideal place for comment.

Also, the name of this test is not informative. Are you testing a valid case or an invalid case? What test case is this?

The test case should be valid because the category id is 1 which satisfies the condition.

[mZneit]

@ZhongweiL in this commit b1fd3c77b8ba71c4646504c3298c4f549014b4dd , I ran the tests and 6/13 tests failed. You added a test that requires the field "should discuss" to be set true. This is not a required field.

[ZhongweiL]

@ZhongweiL in this commit b1fd3c7 , I ran the tests and 6/13 tests failed. You added a test that requires the field "should discuss" to be set true. This is not a required field.

If you are referring to the last test case, "should discuss" should not be null when "func fix" is not null and the problem category id is not 1, 2, or 5 according to the db check constraint at line 32 https://gist.github.com/khatchad/09f0c8d1ca6e0f23b0b9bbf5c62ac8f9

[khatchad]

Right. It's required in some cases.

[khatchad]

BTW, it is likely that there are failing test cases. The reason we are doing this is to understand what needs to be fixed in the insertion form. I expect that there will be at least one case that fails until we fix it.

[mZneit]

After the insertion form fixes, the failing test cases are 3/13 now and not related to the data validation. They are conditions on the sha and user not being null as well as the should_discuss constraint above.

[mZneit]

In https://gist.github.com/khatchad/09f0c8d1ca6e0f23b0b9bbf5c62ac8f9 , the table name is commit_categorizations. I don't see this table in the db. Please correct me if I'm wrong. @khatchad

[khatchad]

Fixed.

[khatchad]

All: Please put these tests up on GH Actions or Travis or something so that we can see what is passing and failing. See https://github.com/ponder-lab/Imperative-DL-Study-Web-App/issues/106#issuecomment-914621742.

[mZneit]

I modified the insertion form instance and applied those modifications to tests.py but I still need to commit those changes. I think we need more test cases though.

[khatchad]

It's failing starting here: https://github.com/ponder-lab/Imperative-DL-Study-Web-App/actions/runs/1267420110

[mZneit]

yes it's because, as users, we cannot create a test database. Only an admin in the development database can make the tests run.

[khatchad]

Great. Please have the admin in the dev database make the test runs.

[mZneit]

@tatianacv the user 'be05ffb901b132' is denied access to create a test database. This is the username in the settings.py file. Granting database creation privilege will handle the error we're getting during CI. Thanks.

[khatchad]

Not working.