[refactor] Replace Path.GetTempPath with AbstractTester in tests

[marwannettour]

Refactored unit and integration test classes to inherit from AbstractTester
and use its TestDirectory property instead of the system temp directory.
This ensures automatic cleanup of temporary files and removes dependency
on the system's temporary directory.
Affected classes:

• InventoryComparerPropagateAccessIssuesTests
• LocalApplicationDataManagerTests
• SynchronizationDownloadFinalizerTests
• DataNodeSourcesViewModelTests
• R2UploadDownload_Tests
• R2DownloadResume_Tests

[Copilot]

Pull request overview

Refactors several unit/integration test fixtures to use ByteSync.TestsCommon.AbstractTester and its TestDirectory instead of Path.GetTempPath() / Path.GetTempFileName(), aiming to standardize test filesystem locations.

Changes:

• Updated multiple test classes to inherit from AbstractTester and call CreateTestDirectory() in [SetUp].
• Replaced usages of Path.GetTempPath() / Path.GetTempFileName() with TestDirectory.FullName-based paths.
• Removed bespoke temp directory setup/teardown in at least one fixture in favor of shared test infrastructure.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/ByteSync.Client.UnitTests/ViewModels/Sessions/DataNodes/DataNodeSourcesViewModelTests.cs	Inherit from AbstractTester; use TestDirectory instead of OS temp path for selected folder path.
tests/ByteSync.Client.UnitTests/Services/Configurations/LocalApplicationDataManagerTests.cs	Inherit from AbstractTester; use TestDirectory as the root for portable/debug test directories.
tests/ByteSync.Client.UnitTests/Services/Comparisons/InventoryComparerPropagateAccessIssuesTests.cs	Replace custom temp dir lifecycle with AbstractTester’s TestDirectory.
tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Downloading/SynchronizationDownloadFinalizerTests.cs	Inherit from AbstractTester; generate temp paths under TestDirectory.
tests/ByteSync.Client.IntegrationTests/Services/Communications/Transfers/R2UploadDownload_Tests.cs	Inherit from AbstractTester; create test files/paths under TestDirectory.
tests/ByteSync.Client.IntegrationTests/Services/Communications/Transfers/R2DownloadResume_Tests.cs	Inherit from AbstractTester; create test files/paths under TestDirectory.

Comments suppressed due to low confidence (1)

tests/ByteSync.Client.UnitTests/Services/Configurations/LocalApplicationDataManagerTests.cs:27

• CreateTestDirectory() runs for every test, but several tests in this fixture don't appear to need a real filesystem location. Since AbstractTester doesn't auto-delete TestDirectory, this will create directories that are never removed. Consider either adding a [TearDown] that deletes TestDirectory, or only calling CreateTestDirectory() in the specific tests that need it.

    [SetUp]
    public void SetUp()
    {
        CreateTestDirectory();
        _environmentServiceMock = new Mock<IEnvironmentService>();
        _environmentServiceMock.SetupGet(e => e.ExecutionMode).Returns(ExecutionMode.Regular);
        _environmentServiceMock.SetupProperty(e => e.Arguments, []);
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The previous implementation created and deleted a unique temp folder per test via [TearDown]. After switching to AbstractTester you no longer delete the created TestDirectory, and AbstractTester itself does not implement a [TearDown] cleanup. This will leave per-test directories under the user's profile and can accumulate over time (especially in CI). Consider reintroducing a [TearDown] here to delete TestDirectory (or otherwise ensure cleanup).

Suggested change

	[TearDown]
	public void TearDown()
	{
	if (!string.IsNullOrWhiteSpace(_tempDirectory) && Directory.Exists(_tempDirectory))
	{
	Directory.Delete(_tempDirectory, true);
	}
	}

[marwannettour]

Fixed. Added a [TearDown] method that deletes TestDirectory after each test, preventing zip inventory files from accumulating under the user profile across runs.

[Copilot]

This fixture now creates a unique TestDirectory for every test in SetUp, but there is no corresponding cleanup and the directory isn't actually needed for the current assertions (the dialog result is only used to verify mock calls). Since AbstractTester doesn't delete TestDirectory automatically, this can leave behind many empty directories. Consider removing CreateTestDirectory() here (and using a stable path string), or add cleanup in [TearDown].

Suggested change

CreateTestDirectory();

[Copilot]

Now that temp files are created under TestDirectory (in the user's profile) rather than the OS temp location, the test-created files/directories are less likely to be cleaned up by the system. This fixture calls CreateTestDirectory() but never deletes TestDirectory in TearDown, so large artifacts (e.g., the 1MB temp file and any download outputs) will accumulate across runs. Consider deleting TestDirectory in [TearDown] once the test completes.

[marwannettour]

After reviewing the file, TestDirectory is created but never actually written to .It is only used as a string value passed to a mocked dialog service (ShowOpenFolderDialogAsync). No files are ever created inside it, so the directory remains empty after each test. The accumulation impact is negligible (empty directories only), so no teardown cleanup was added here.

[Copilot]

Now that temp files are created under TestDirectory (in the user's profile) rather than the OS temp location, the test-created files/directories are less likely to be cleaned up by the system. This fixture calls CreateTestDirectory() but never deletes TestDirectory in TearDown, so downloaded/uploaded artifacts may accumulate across runs. Consider deleting TestDirectory in [TearDown] once the test completes.

[marwannettour]

Fixed. The existing [TearDown] already disposed _clientScope; added TestDirectory.Delete(true) to also clean up the test directory, preventing upload/download artifacts (including the 1 MB temp file) from accumulating under the user profile across runs.

[sonarqubecloud]

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

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

CreateTestDirectory() creates a new directory under the user profile (via AbstractTester) but this fixture never deletes it. Since AbstractTester has no built-in [TearDown] cleanup, these per-test directories will accumulate across runs. Add a [TearDown] that deletes TestDirectory (with an Exists check), or stop creating a test directory here if it’s only used as a string for mocking.

[Copilot]

CreateTestDirectory() allocates a directory under %USERPROFILE%/ByteSync_Automated_Tests/..., but this fixture does not delete TestDirectory after tests that don’t remove tempRoot manually. Because AbstractTester does not auto-cleanup, add a fixture-level [TearDown] that deletes TestDirectory to avoid leaving test artifacts behind.

[Copilot]

This fixture creates a new TestDirectory in [SetUp] for every test, but there is no corresponding deletion. AbstractTester does not implement teardown cleanup, so these per-test directories (and any temp files like zips) will accumulate. Consider extending the existing [TearDown] to delete TestDirectory (guarded by Exists).

[Copilot]

CreateTestDirectory() creates a directory under the user profile, but [TearDown] only disposes _clientScope. Since this test writes temp files under TestDirectory, add deletion of TestDirectory (with an Exists check) in [TearDown] to prevent leftover artifacts across runs.

[Copilot]

The PR description says inheriting from AbstractTester "ensures automatic cleanup", but AbstractTester.CreateTestDirectory() only creates a directory and provides no teardown cleanup. Either update the PR description, or ensure each fixture deletes TestDirectory in [TearDown] consistently.

[marwannettour]

OK