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

Move downloads out of Temp and clean up after cancel and complete #1704

Merged
merged 5 commits into from
Jul 2, 2024

Conversation

Al12rs
Copy link
Contributor

@Al12rs Al12rs commented Jul 1, 2024

Ongoing download files are now stored next to DataModel folder, in Downloads/Ongoing.
Each download task will have a separate folder (guid name), which simplifies cleanup on download cancellation and download completion.

The base folder for the Downloads is determined by a setting, but it can be overruled through an internal method for testing purposes (so that each test has a different download base folder).

@Al12rs Al12rs requested a review from erri120 July 1, 2024 12:02
@Al12rs Al12rs self-assigned this Jul 1, 2024
# Conflicts:
#	tests/Networking/NexusMods.Networking.Downloaders.Tests/DownloadServiceDataStoreTests.cs
@Al12rs
Copy link
Contributor Author

Al12rs commented Jul 1, 2024

Need to look into the test not passing on windows
EDIT: Test appears to be flakey

@Al12rs Al12rs requested a review from erri120 July 1, 2024 14:39
@Al12rs
Copy link
Contributor Author

Al12rs commented Jul 2, 2024

I will change the tests to use a Setting override, since it should actually be better for running tests concurrently

…ncurrency problems of switching base folder of the same service.
@Al12rs Al12rs requested a review from erri120 July 2, 2024 08:56
@Al12rs
Copy link
Contributor Author

Al12rs commented Jul 2, 2024

Used the setting override in the tests, tests pass locally and it doesn't seem like any files are left around after the test (the folder structure yes, but it is empty).

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 91.42857% with 6 lines in your changes missing coverage. Please review.

Project coverage is 40.22%. Comparing base (cdfcc2d) to head (5feae1f).
Report is 16 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1704      +/-   ##
==========================================
- Coverage   42.15%   40.22%   -1.93%     
==========================================
  Files         769      706      -63     
  Lines       26393    24873    -1520     
  Branches     1999     1885     -114     
==========================================
- Hits        11126    10006    -1120     
+ Misses      14895    14533     -362     
+ Partials      372      334      -38     
Flag Coverage Δ
Windows 40.09% <88.57%> (?)
clean_environment_tests 40.20% <90.00%> (-1.95%) ⬇️
macOS 39.44% <88.57%> (-2.72%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...exusMods.Networking.Downloaders/DownloadService.cs 55.17% <100.00%> (+2.96%) ⬆️
...rking/NexusMods.Networking.Downloaders/Services.cs 100.00% <100.00%> (+9.09%) ⬆️
...Mods.Networking.Downloaders/Tasks/ADownloadTask.cs 82.71% <100.00%> (+1.50%) ⬆️
...ds.Networking.HttpDownloader/DTOs/DownloadState.cs 100.00% <100.00%> (ø)
...ns/DownloadStatus/DownloadStatusDesignViewModel.cs 76.78% <ø> (+1.34%) ⬆️
...etworking.HttpDownloader/AdvancedHttpDownloader.cs 70.69% <80.00%> (ø)
...ttpDownloader.Tests/AdvancedHttpDownloaderTests.cs 0.00% <0.00%> (ø)
...xusMods.Networking.Downloaders/DownloadSettings.cs 84.61% <84.61%> (ø)

... and 65 files with indirect coverage changes

@Al12rs Al12rs merged commit 0f6e088 into main Jul 2, 2024
11 checks passed
@Al12rs Al12rs deleted the fix/preserve-ongoing-downloads branch July 2, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Closing App cancels all ongoing downloads
3 participants