[GSoC2024] - add/rest-api-tests-for-attribute-rename #7754
Conversation
|
Hi @Marishka17 @nmanovic |
|
Hi @azhavoro, |
tests/python/rest_api/test_labels.py
Outdated
| label = next( | ||
| iter( | ||
| self._labels_by_source( | ||
| self.labels, source_key=self._get_source_info(source).label_source_key | ||
| ).values() | ||
| ) | ||
| )[0] | ||
|
|
||
| if label.get("attributes"): |
There was a problem hiding this comment.
Only a label with attributes should be selected here. Overwise the test will always pass if a label has no attributes and there will be no point in such a test.
tests/python/rest_api/test_labels.py
Outdated
| newvalue = label.get("attributes") | ||
|
|
||
| for value in newvalue: | ||
| if value.get("id"): |
There was a problem hiding this comment.
Since a label with attributes is selected from existing ones, each attribute must contain id.
tests/python/rest_api/test_labels.py
Outdated
| if "attributes.id" in ignore_fields: | ||
| ignore_fields.remove("attributes.id") |
There was a problem hiding this comment.
This should be handled in _get_patch_data function.
|
@Marishka17 |
tests/python/rest_api/test_labels.py
Outdated
| def _labels_with_attributes(labels: List[Dict], *,source_key: str) -> Dict[int, Dict]: | ||
| labels_with_attributes = {} | ||
| for label in labels: | ||
| label_source = label.get(source_key) | ||
| if label_source and label.get("attributes"): | ||
| labels_with_attributes.setdefault(label_source, []).append(label) | ||
|
|
||
| return labels_with_attributes |
There was a problem hiding this comment.
IMHO, such a function should just keep labels with attributes and return a filtered list (without filtering by source_key). But I don't see now the real necessity to move this logic in a separate function.
There was a problem hiding this comment.
I have removed the function.
tests/python/rest_api/test_labels.py
Outdated
| ] | ||
| filtered_attributes = self._attributes_with_id(deepcopy(original_data.get("attributes", []))) | ||
|
|
||
| if overrides.get("attributes")[0].get("id") is None: |
There was a problem hiding this comment.
Probably, it is not a good approach to rely only on the first attribute from the list. You can add some argument (which can determine whether new attributes are being added or updated) to _get_patch_data and rely on it.
tests/python/rest_api/test_labels.py
Outdated
| param = "attributes" | ||
| newvalue = label.get("attributes") | ||
|
|
||
| for value in newvalue: | ||
| value.update({"name": value["name"] + "_updated"}) | ||
|
|
||
| expected_data, patch_data, ignore_fields = self._get_patch_data(label, **{param: newvalue}) |
There was a problem hiding this comment.
| param = "attributes" | |
| newvalue = label.get("attributes") | |
| for value in newvalue: | |
| value.update({"name": value["name"] + "_updated"}) | |
| expected_data, patch_data, ignore_fields = self._get_patch_data(label, **{param: newvalue}) | |
| attributes = label["attributes"] | |
| for attribute in attributes: | |
| attribute["name"] += "_updated" | |
| expected_data, patch_data, ignore_fields = self._get_patch_data(label, attributes=attributes) |
tests/python/rest_api/test_labels.py
Outdated
| payload["attributes"] = (original_data.get("attributes") or []) + overrides[ | ||
| "attributes" | ||
| ] | ||
| filtered_attributes = self._attributes_with_id(deepcopy(original_data.get("attributes", []))) |
There was a problem hiding this comment.
Why do we need to use _attributes_with_id here? Since original_data represents a created label taken from assets, then if a label contains attributes -> each attribute has id.
There was a problem hiding this comment.
I removed this extra step.
tests/python/rest_api/test_labels.py
Outdated
| @@ -656,6 +688,30 @@ def test_can_patch_label_field(self, source, admin_user, param, newvalue): | |||
| ).values() | |||
| ) | |||
| )[0] | |||
| print('label', label) | |||
There was a problem hiding this comment.
please remove the debug code
|
@Marishka17 |
Added rest-api tests for attribute rename feature.
Motivation and context
These rest-api tests are to validate the changes introduced in the PR here
How has this been tested?
Checklist
developbranch- [ ] I have created a changelog fragment- [ ] I have updated the documentation accordingly- [ ] I have linked related issues (see GitHub docs)- [ ] I have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.