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

Extend server tests for dataset manager. #3192

Merged
merged 54 commits into from
Jun 22, 2021

Conversation

MashaSS
Copy link
Contributor

@MashaSS MashaSS commented May 14, 2021

Extend server tests for pull request #3004.

Dmitriy Oparin and others added 26 commits March 12, 2021 09:49
Commented out tests passed Ok
Uncommented tests failed if launched in single test session, seems that tasks created via POST request to url /api/v1/tasks have the same id (1)
…zuevx/cvat into ms/server_test_dataset_manager

# Conflicts:
#	kubernetes-templates/03_database_deployment.yml
#	kubernetes-templates/03_redis_deployment.yml
#	kubernetes-templates/04_cvat_backend_deployment.yml
#	kubernetes-templates/04_cvat_frontend_deployment.yml
#	kubernetes-templates/04_database_service.yml
#	kubernetes-templates/04_redis_service.yml
#	kubernetes-templates/05_cvat_backend_service.yml
#	kubernetes-templates/05_cvat_frontend_service.yml
#	kubernetes-templates/05_cvat_proxy_deployment.yml
#	kubernetes-templates/05_cvat_proxy_service.yml
#	kubernetes-templates/README.md
Comment on lines +375 to +377
content = BytesIO(b"".join(response.streaming_content))
with open(file_zip_name, "wb") as f:
f.write(content.getvalue())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
content = BytesIO(b"".join(response.streaming_content))
with open(file_zip_name, "wb") as f:
f.write(content.getvalue())
content = b"".join(response.streaming_content)
with open(file_zip_name, "wb") as f:
f.write(content)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't work that way.

task_ann_data = task_ann.data
self.assertEqual(len(task_ann_data["shapes"]), len(task_ann_prev_data["shapes"]))

def test_api_v1_unit_test_on_normalize_shape_function(self):
Copy link
Contributor

@zhiltsov-max zhiltsov-max Jun 9, 2021

Choose a reason for hiding this comment

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

What does this test test? Also, I see repeated cases with different comments. Also, this test does not use rest api, maybe its better place in test_annotations.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, what about other problems with this test? Currently, it only checks for not throwing an exception. It has 2 repeated test cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Repeated tests fixed and check length of returned values.

dump_format_name = dump_format.DISPLAY_NAME
with self.subTest():
if dump_format_name in [
"MOT 1.1", # issue #2925
Copy link
Contributor

Choose a reason for hiding this comment

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

@MashaSS , I see that some of these problems were fixed but tests are disabled. Could you please re-check?

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.

MOT 1.1 and MOTS PNG 1.0 cannot be checked by datumaro. I removed old comments about this issue.

from cvat.apps.dataset_manager.bindings import CvatTaskDataExtractor, TaskData
from cvat.apps.dataset_manager.task import TaskAnnotation
from cvat.apps.dataset_manager.annotation import TrackManager
Copy link
Contributor

Choose a reason for hiding this comment

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

@MashaSS , the import is unused for now. It leads to a warning from pylint. Could you please fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

"name": "camera_id",
"mutable": false,
"input_type": "number",
"values": ["1", "2", "3", "4", "5", "6"]
Copy link
Contributor

Choose a reason for hiding this comment

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

@MashaSS , could you please help me to recall? Why do we have a list of values for "text" and "number"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Values for "number" define range of possible values (min, max, step). Values for "text" are used in requests but don't define any value restrictions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Values for text should be text. Do you agree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, could you please clarify what test values should be changed? Attributes color and center in icdar_segmentation task?

def test_api_v1_unit_test_on_normalize_shape_function(self):
# 3 points
norm_shape = TrackManager.normalize_shape({
Copy link
Contributor

Choose a reason for hiding this comment

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

@MashaSS , @yasakova-anastasia , why do we test "normalize_shape" function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the part of code related to dataset manager. It was decided to place it in test_annotation.py because test doesn't use rest api.

Copy link
Contributor

@nmanovic nmanovic Jun 21, 2021

Choose a reason for hiding this comment

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

@MashaSS , the function should be removed in the code. It is dead code. No need to cover it by tests. Just remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed dead code function and test for this function.

url = self._generate_url_upload_tasks_annotations(task_id, upload_format_name)

with open(file_zip_name, 'rb') as binary_file:
response = self._put_request_with_data(url, {"annotation_file": binary_file}, user)
Copy link
Contributor

Choose a reason for hiding this comment

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

@MashaSS , I miss real checks in the test. Basically you create a task, create some annotations, dump the file. The same for upload is done. But the test doesn't check that uploaded and dumped information corresponds to each over.

@yasakova-anastasia , need to design tests which are going to check real data. Not just HTTP codes.

@nmanovic nmanovic merged commit c730920 into cvat-ai:develop Jun 22, 2021
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.

6 participants