8000 Fix `test_tasks.py::TestImportTaskAnnotations::test_check_import_error_on_wrong_file_structure` by Marishka17 · Pull Request #9303 · cvat-ai/cvat · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix test_tasks.py::TestImportTaskAnnotations::test_check_import_error_on_wrong_file_structure #9303

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Apr 10, 2025
Merged

Fix test_tasks.py::TestImportTaskAnnotations::test_check_import_error_on_wrong_file_structure #9303

merged 4 commits into from
Apr 10, 2025

Conversation

Marishka17
Copy link
Contributor
@Marishka17 Marishka17 commented Apr 3, 2025 •
edited
Loading

Motivation and context

There were several issues with this test, like:

  • the archive being created was not closed
  • exception capture details were never checked
  • not all formats return expected error format

How has this been tested?

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues (see GitHub docs)

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.

@Marishka17 Marishka17 requested a review from zhiltsov-max as a code owner April 3, 2025 11:24
@Marishka17 Marishka17 force-pushed the mk/fix_test_check_import_error_on_wrong_file_structure branch from dbd3114 to 231edc4 Compare April 3, 2025 11:34
@Marishka17 Marishka17 force-pushed the mk/fix_test_check_import_error_on_wrong_file_structure branch from 231edc4 to 231d0f6 Compare April 3, 2025 11:50
@Marishka17 Marishka17 mentioned this pull request Apr 3, 2025
Open
6 tasks
Marishka17
Marishka17 commented Apr 3, 2025
View reviewed changes
cvat/apps/dataset_manager/formats/cvat.py
@@ -1674,8 +1674,8 @@ def _import(src_file, temp_dir, instance_data, load_data_callback=None, **kwargs
if is_zip:
zipfile.ZipFile(src_file).extractall(temp_dir)

detect_dataset(temp_dir, format_name="cvat", importer=_CvatImporter)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see the reason why we should check the uploaded file structure only when importing into a project

Copy link
Contributor

Choose a reason for hiding this comment

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

The change looks reasonable to me, but since it's user-facing, you should add a changelog entry.

@codecov-commenter
Copy link
codecov-commenter commented Apr 3, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.77%. Comparing base (2f110e5) to head (048f3e2).
Report is 28 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #9303      +/-   ##
===========================================
- Coverage    74.17%   73.77%   -0.41%     
===========================================
  Files          428      436       +8     
  Lines        44964    45551     +587     
  Branches      3917     3920       +3     
===========================================
+ Hits         33352    33603     +251     
- Misses       11612    11948     +336
Components Coverage Δ
cvat-ui 77.22% <ø> (-0.02%) ⬇️
cvat-server 70.96% <85.71%> (-0.67%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Marishka17 Marishka17 requested review from SpecLad and removed request for zhiltsov-max April 3, 2025 13:12
SpecLad
SpecLad reviewed Apr 3, 2025
View reviewed changes
@Marishka17 Marishka17 requested a review from nmanovic as a code owner April 10, 2025 08:20
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud
8000

@SpecLad SpecLad merged commit 74f30f1 into develop Apr 10, 2025
34 checks passed
@SpecLad SpecLad deleted the mk/fix_test_check_import_error_on_wrong_file_structure branch April 10, 2025 09:48
@cvat-bot cvat-bot bot mentioned this pull request Apr 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SpecLad SpecLad SpecLad approved these changes

@nmanovic nmanovic Awaiting requested review from nmanovic nmanovic is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Marishka17 @codecov-commenter @SpecLad
0