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

Improve REST API testing system #4403

Merged
merged 40 commits into from
Mar 16, 2022
Merged

Improve REST API testing system #4403

merged 40 commits into from
Mar 16, 2022

Conversation

sizov-kirill
Copy link
Contributor

@sizov-kirill sizov-kirill commented Mar 1, 2022

Motivation and context

The current test database initialization strategy for REST API tests uses the cvat_db.sql file, this approach makes it difficult to develop tests in parallel because it is not clear how to perform *.sql file merging. So this PR solve this problem with Django functionality.

This PR suggest to initialize test db in this way:

#Dump
docker exec cvat python manage.py dumpdata --indent 2 >  tests/rest_api/assets/cvat_db/data.json

#Restore
docker container cp tests/rest_api/assets/cvat_db/data.json cvat:data.json
docker exec cvat python manage.py loaddata /data.json

Increase time with new recovery strategy: ~4 sec.

  • Change confest.py according new approach of initializing test db.
  • Update JSON assets
  • Update tests according to new changes
  • Estimate convenience of merging two branches that update test db.
  • Reorganize assets directory
  • Delete all things that relates with old testing system
  • Update README

How has this been tested?

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2022 Intel Corporation
#
# SPDX-License-Identifier: MIT

@sizov-kirill sizov-kirill changed the title [WIP] Improve REST API testing system Improve REST API testing system Mar 2, 2022
@sizov-kirill sizov-kirill changed the title Improve REST API testing system [WIP] Improve REST API testing system Mar 2, 2022
@sizov-kirill sizov-kirill changed the title [WIP] Improve REST API testing system Improve REST API testing system Mar 5, 2022
cvat_db_container('psql -U root -q -d test_db -f /cvat_db/cvat_db.sql')
_run(f"docker container cp {osp.join(CVAT_DB_DIR, 'restore.sh')} cvat_db:restore.sh")
_run(f"docker container cp {osp.join(CVAT_DB_DIR, 'data.json')} cvat:data.json")
_run('docker exec cvat python manage.py loaddata /data.json')
Copy link
Contributor

Choose a reason for hiding this comment

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

psql command supports -v option which allows you to set a variable. Thus you can write cvat_db.sql script with :src_db and :dst_db and set them in cvat_db.sql script. Thus you don't need restore.sh script. More details here: https://stackoverflow.com/questions/36959/how-do-you-use-script-variables-in-psql

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -0,0 +1,5 @@
restore="SELECT pg_terminate_backend(pg_stat_activity.pid) FROM pg_stat_activity WHERE pg_stat_activity.datname = 'cvat' AND pid <> pg_backend_pid();\
Copy link
Contributor

@nmanovic nmanovic Mar 9, 2022

Choose a reason for hiding this comment

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

It should be replaced by restore_db.sql with variables (see psql -v)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@nmanovic
Copy link
Contributor

nmanovic commented Mar 9, 2022

If I run tests on a non-empty CVAT instance, it will fail. What do you recommend?

(cvat) nmanovic@nmanovic-pc:~/Workspace/cvat$ pytest tests/rest_api/ -s
====================================================== test session starts =======================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
rootdir: /mnt/data.ssd1tb/workspace/cvat
collected 162 items                                                                                                              

tests/rest_api/test_analytics.py Traceback (most recent call last):
  File "/opt/venv/lib/python3.8/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "authtoken_token_user_id_key"
DETAIL:  Key (user_id)=(10) already exists.


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "manage.py", line 21, in <module>
    execute_from_command_line(sys.argv)
  File "/opt/venv/lib/python3.8/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "/opt/venv/lib/python3.8/site-packages/django/core/management/__init__.py", line 413, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/opt/venv/lib/python3.8/site-packages/django/core/management/base.py", line 354, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/opt/venv/lib/python3.8/site-packages/django/core/management/base.py", line 398, in execute
    output = self.handle(*args, **options)
  File "/opt/venv/lib/python3.8/site-packages/django/core/management/commands/loaddata.py", line 78, in handle
    self.loaddata(fixture_labels)
  File "/opt/venv/lib/python3.8/site-packages/django/core/management/commands/loaddata.py", line 123, in loaddata
    self.load_label(fixture_label)
  File "/opt/venv/lib/python3.8/site-packages/django/core/management/commands/loaddata.py", line 190, in load_label
    obj.save(using=self.using)
  File "/opt/venv/lib/python3.8/site-packages/django/core/serializers/base.py", line 223, in save
    models.Model.save_base(self.object, using=using, raw=True, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/django/db/models/base.py", line 776, in save_base
    updated = self._save_table(
  File "/opt/venv/lib/python3.8/site-packages/django/db/models/base.py", line 881, in _save_table
    results = self._do_insert(cls._base_manager, using, fields, returning_fields, raw)
  File "/opt/venv/lib/python3.8/site-packages/django/db/models/base.py", line 919, in _do_insert
    return manager._insert(
  File "/opt/venv/lib/python3.8/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/django/db/models/query.py", line 1270, in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
  File "/opt/venv/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 1416, in execute_sql
    cursor.execute(sql, params)
  File "/opt/venv/lib/python3.8/site-packages/django/db/backends/utils.py", line 66, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/opt/venv/lib/python3.8/site-packages/django/db/backends/utils.py", line 75, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/opt/venv/lib/python3.8/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/opt/venv/lib/python3.8/site-packages/django/db/utils.py", line 90, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/opt/venv/lib/python3.8/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.IntegrityError: Problem installing fixture '/data.json': Could not load authtoken.Token(pk=3952f1aea900fc3daa269473a71c41fac08858b5): duplicate key value violates unique constraint "authtoken_token_user_id_key"
DETAIL:  Key (user_id)=(10) already exists.



! _pytest.outcomes.Exit: Command failed: docker exec cvat python manage.py loaddata /data.json. Add `-s` option to see more details !

@nmanovic
Copy link
Contributor

nmanovic commented Mar 9, 2022

@kirill-sizov , I have a couple of questions:

  • The PR remotes some directories from cvat_data (e.g., tasks/*). Are you sure that we don't need them? In general a task directory can have raw data, original data, compressed data and some auxiliary files.
  • How does unziping cvat_data help to resolve conflicts? If somebody creates a task in two parallel PRs, they will have the same primary key (for data, task and jobs). In this case it will be necessary to resolve the conflict manually. Am I right? If so, will it be better to have cvat_data as an archive?

@sizov-kirill
Copy link
Contributor Author

sizov-kirill commented Mar 10, 2022

@kirill-sizov , I have a couple of questions:

  • The PR remotes some directories from cvat_data (e.g., tasks/*). Are you sure that we don't need them? In general a task directory can have raw data, original data, compressed data and some auxiliary files.
  • How does unziping cvat_data help to resolve conflicts? If somebody creates a task in two parallel PRs, they will have the same primary key (for data, task and jobs). In this case it will be necessary to resolve the conflict manually. Am I right? If so, will it be better to have cvat_data as an archive?
  1. No problem to store other directories if needed, but directory data is enough for now.

  2. You right, the conflict that include same ids should be fixed manually. Storing in unzipped format allows easy to see what changes was maid in commit.

@sizov-kirill
Copy link
Contributor Author

sizov-kirill commented Mar 10, 2022

@nmanovic

psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "authtoken_token_user_id_key"
DETAIL:  Key (user_id)=(10) already exists.

I don't exactly know the cause of this problem, but in my case this problem only occurs if my current database was built not with loaddata. To resolve this problem, you can recreate your database:

docker exec cvat_db psql -d postgres -v from:test_db to:cvat -f restore.sql
docker exec cvat_db createdb cvat
docker exec cvat python manage.py migrate

@nmanovic
Copy link
Contributor

@nmanovic

psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "authtoken_token_user_id_key"
DETAIL:  Key (user_id)=(10) already exists.

I don't exactly know the cause of this problem, but in my case this problem only occurs if my current database was not built with loaddata. To resolve this problem, you can recreate your database:

docker exec cvat_db psql -d postgres -v from:test_db to:cvat -f restore.sql
docker exec cvat_db createdb cvat
docker exec cvat python manage.py migrate

I believe it is because I already have cvat db and it isn't empty. Probably we need to run tests on a clear CVAT instance. There are two ways: say about that in documentation, drop cvat db before start. Need to choose the best solution.

@sizov-kirill
Copy link
Contributor Author

@nmanovic

  1. I decided to keep tar format for data volumes. But we can easily to change this in future, if we will find out some new reasons for storing data volumes in unzipped format.

  2. The problem with exists key error do not occurs in new testing system. This means if your database was create by loaddata then tests will run successfully. For other case need to re-create database as it mentioned in README. If this error will occurs systematically in different cases, I will fix this problem with drop current database before running tests.

@nmanovic
Copy link
Contributor

@kirill-sizov , I have merged the PR from Andrey. Could you please resolve conflicts?

@sizov-kirill
Copy link
Contributor Author

@kirill-sizov , I have merged the PR from Andrey. Could you please resolve conflicts?

Done

cat assets/cvat_db/cvat_db.sql | docker exec -i cvat_db psql -U root -d cvat
cat assets/cvat_data.tar.bz2 | docker run --rm -i --volumes-from cvat ubuntu tar -xj --strip 3 -C /home/django/data
docker container cp assets/cvat_db/data.json cvat:data.json
docker exec cvat python manage.py loaddata /data.json
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

- `cvat_data.tar.bz2` --- archieve with data volumes;
- `data.json` --- file required for DB restoring.
Contains all information about test db;
- `restore.sh` --- simple bash script for creating copy of database and
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still have restore.sh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

In this case you should terminate all existent connections for cvat database,
you can perform it with command:
```
docker exec cvat_db sh restore.sh cvat test_db
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have restore.sh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@nmanovic
Copy link
Contributor

@kirill-sizov , I see cvat_data.tar.bz2 twice in the PR.

@nmanovic
Copy link
Contributor

@kirill-sizov , could you please double check your PR next time?

@sizov-kirill
Copy link
Contributor Author

@kirill-sizov , I see cvat_data.tar.bz2 twice in the PR.

I see only one cvat_data.tar.bz2:

tree tests/rest_api/ -P *tar.bz2
tests/rest_api/
├── assets
│   └── cvat_db
│       └── cvat_data.tar.bz2
└── utils

@nmanovic nmanovic merged commit 6dd73b0 into develop Mar 16, 2022
@nmanovic nmanovic deleted the sk/improve-testing-system branch March 16, 2022 08:49
mikhail-treskin pushed a commit to retailnext/cvat that referenced this pull request Jun 22, 2022
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.

2 participants