-
Notifications
You must be signed in to change notification settings - Fork 774
Fixes #5158. Refactor FileDialog base type from Dialog to Dialog<IReadOnlyList<string>?>
#5160
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
b803654
Initial plan
Copilot 0866596
Refactor FileDialog base type from Dialog to Dialog<IReadOnlyList<str…
Copilot 5eb9e1e
Address review feedback: fix test naming and unused parameter
Copilot 102ef3a
Potential fix for pull request finding
tig e4550e9
Potential fix for pull request finding
tig 967d0b6
Fix CI build error: use bool? for InvokeCommand return type and clear…
Copilot 106e296
Add test proving Cancel clears Result when dialog was previously acce…
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
173 changes: 173 additions & 0 deletions
173
Tests/UnitTestsParallelizable/Views/FileDialogResultTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,173 @@ | ||
| // Copilot | ||
|
|
||
| using System.IO.Abstractions.TestingHelpers; | ||
|
|
||
| namespace UnitTests.Views; | ||
|
|
||
| /// <summary> | ||
| /// Tests for <see cref="FileDialog"/> refactored to inherit from <see cref="Dialog{TResult}"/> | ||
| /// where TResult is <c>IReadOnlyList<string>?</c>. | ||
| /// </summary> | ||
| public class FileDialogResultTests | ||
| { | ||
| [Fact] | ||
| public void FileDialog_Result_IsNull_WhenNotAccepted () | ||
| { | ||
| // Arrange | ||
| MockFileSystem fs = new (); | ||
| fs.AddDirectory ("/testdir"); | ||
| using FileDialog fd = new TestableFileDialog (fs); | ||
|
|
||
| // Assert - Result should be null before any acceptance | ||
| Assert.Null (fd.Result); | ||
| Assert.True (fd.Canceled); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void FileDialog_Result_IsNull_BeforeAcceptance () | ||
| { | ||
| // Arrange | ||
| MockFileSystem fs = new (); | ||
| fs.AddFile ("/testdir/file1.txt", new MockFileData ("hello")); | ||
| using SaveDialog sd = new TestableSaveDialog (fs); | ||
| sd.Path = "/testdir/file1.txt"; | ||
|
|
||
| // Assert - Result should be null before any acceptance | ||
| Assert.True (sd.Canceled); | ||
| Assert.Null (sd.Result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void FileDialog_Canceled_IsTrue_WhenResultIsNull () | ||
| { | ||
| // Arrange | ||
| MockFileSystem fs = new (); | ||
| fs.AddDirectory ("/testdir"); | ||
| using FileDialog fd = new TestableFileDialog (fs); | ||
|
|
||
| // Assert | ||
| Assert.True (fd.Canceled); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void FileDialog_Canceled_IsFalse_WhenAccepted_ThroughCommandPipeline () | ||
| { | ||
| // Arrange | ||
| MockFileSystem fs = new (); | ||
| fs.AddFile ("/testdir/file1.txt", new MockFileData ("hello")); | ||
| using SaveDialog sd = new TestableSaveDialog (fs); | ||
| sd.Path = "/testdir/file1.txt"; | ||
|
|
||
| // Act | ||
| bool? accepted = sd.InvokeCommand (Command.Accept); | ||
|
|
||
| // Assert | ||
| Assert.True (accepted is true); | ||
| Assert.False (sd.Canceled); | ||
| Assert.NotNull (sd.Result); | ||
| Assert.Single (sd.Result); | ||
| Assert.Equal ("/testdir/file1.txt", sd.Result [0]); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void FileDialog_InheritsFromDialogOfReadOnlyListString () | ||
| { | ||
| // Arrange | ||
| MockFileSystem fs = new (); | ||
| fs.AddDirectory ("/testdir"); | ||
| using FileDialog fd = new TestableFileDialog (fs); | ||
|
|
||
| // Assert - verify the new base type | ||
| Assert.IsAssignableFrom<Dialog<IReadOnlyList<string>?>> (fd); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void OpenDialog_FilePaths_IsEmpty_WhenCanceled () | ||
| { | ||
| // Arrange | ||
| using OpenDialog od = new TestableOpenDialog (); | ||
|
|
||
| // Assert - Result is null → Canceled → FilePaths empty | ||
| Assert.True (od.Canceled); | ||
| Assert.Empty (od.FilePaths); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void SaveDialog_FileName_IsNull_WhenCanceled () | ||
| { | ||
| // Arrange | ||
| MockFileSystem fs = new (); | ||
| fs.AddDirectory ("/testdir"); | ||
| using SaveDialog sd = new TestableSaveDialog (fs); | ||
|
|
||
| // Assert | ||
| Assert.True (sd.Canceled); | ||
| Assert.Null (sd.FileName); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void FileDialog_Result_MultiSelection_IsPopulated () | ||
| { | ||
| // Arrange | ||
| MockFileSystem fs = new (); | ||
| fs.AddFile ("/testdir/file1.txt", new MockFileData ("a")); | ||
| fs.AddFile ("/testdir/file2.txt", new MockFileData ("b")); | ||
| using FileDialog fd = new TestableFileDialog (fs); | ||
|
|
||
| // Act - directly set Result as would happen after multi-select acceptance | ||
| List<string> paths = ["/testdir/file1.txt", "/testdir/file2.txt"]; | ||
| fd.Result = paths.AsReadOnly (); | ||
|
|
||
| // Assert | ||
| Assert.False (fd.Canceled); | ||
| Assert.Equal (2, fd.Result.Count); | ||
| Assert.Contains ("/testdir/file1.txt", fd.Result); | ||
| Assert.Contains ("/testdir/file2.txt", fd.Result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void FileDialog_Cancel_ClearsResult_WhenPreviouslyAccepted () | ||
| { | ||
| // Copilot | ||
| // This test validates that canceling a dialog after a previous acceptance | ||
| // correctly clears Result to null (so Canceled == true). | ||
| // Without the explicit `Result = null` in the Cancel path, this test would fail | ||
| // because Result would retain the previously accepted value. | ||
|
|
||
| // Arrange | ||
| MockFileSystem fs = new (); | ||
| fs.AddFile ("/testdir/file1.txt", new MockFileData ("hello")); | ||
| using SaveDialog sd = new TestableSaveDialog (fs); | ||
| sd.Path = "/testdir/file1.txt"; | ||
|
|
||
| // Act - first accept the dialog to set Result | ||
| sd.InvokeCommand (Command.Accept); | ||
| Assert.False (sd.Canceled); // Result is set — not canceled | ||
|
|
||
| // Act - now simulate pressing Cancel button | ||
| Button cancelBtn = sd.Buttons [sd.CancelButtonIndex]; | ||
| cancelBtn.InvokeCommand (Command.Accept); | ||
|
|
||
| // Assert - Result should be cleared, Canceled should be true | ||
| Assert.Null (sd.Result); | ||
| Assert.True (sd.Canceled); | ||
| } | ||
|
|
||
| /// <summary>Testable subclass that exposes the internal file-system constructor.</summary> | ||
| private sealed class TestableFileDialog : FileDialog | ||
| { | ||
| public TestableFileDialog (MockFileSystem fs) : base (fs) { } | ||
| } | ||
|
|
||
| /// <summary>Testable subclass for OpenDialog.</summary> | ||
| private sealed class TestableOpenDialog : OpenDialog | ||
| { | ||
| public TestableOpenDialog () { } | ||
| } | ||
|
|
||
| /// <summary>Testable subclass for SaveDialog.</summary> | ||
| private sealed class TestableSaveDialog : SaveDialog | ||
| { | ||
| public TestableSaveDialog (MockFileSystem fs) : base (fs) { } | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply changes based on this feedback
Ensure there's a test that fails befor fixing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in
106e296. AddedFileDialog_Cancel_ClearsResult_WhenPreviouslyAcceptedtest that:Resultto["/testdir/file1.txt"])ResultisnullandCanceledistrueVerified the test fails without the
Result = nullline (Result retains the previously accepted value["/testdir/file1.txt"]) and passes with it.