TemporaryFolder: Remove unnecessary call to TemporaryFolder#newFile #1009
Conversation
|
Click here to submit a new iteration when this PR is ready for review. See this repository's contribution guide for more information. |
TemporaryFolder#newFile LogicManagerTest and MainWindowCloseTest calls newFile() on a temporary folder to initialize an empty file for the AddressBoook and UserPrefs storage. This is unnecessary, as the file is automatically created the first time it is written to. Instead of explicitly creating a new file, let's simply reference a file path with Path#resolve instead.
sijie123
changed the title
v1@sijie123 submitted v1 for review. Checkout this PR version locallygit fetch https://github.com/se-edu/addressbook-level4.git refs/pr/1009/1/head:BRANCHNAMEwhere |
There was a problem hiding this comment.
LogicManagerTest, MainWindowCloseTest: Remove unnecessary call to TemporaryFolder#newFile LogicManagerTest and MainWindowCloseTest calls newFile() on a temporary folder to initialize an empty file for the AddressBoook and UserPrefs storage. This is unnecessary, as the file is automatically created the first time it is written to. Instead of explicitly creating a new file, let's simply reference a file path with Path#resolve instead.
Eh as a standalone PR this isn't really convincing, especially since the
end result is much more verbose than the original.
The reason why we are doing it is because we want to use JUnit5's
TempDir extension. However, the TempDir extension does not support
newFile() (i.e. a method that creates a file with a random name). Since
we do not want to spend the effort to re-implement newFile(), and the
random file names are actually not needed by the test (since the test
fully controls the contents of the temporary directory), we explicitly
give the files names.
If that's the true reason, then we shouldn't try to hide it, but rather
just state it directly in [v9 11/16] in #959.
Or alternatively (less recommended), put this as a preparatory commit in #959, just before
[v9 11/16], but explicitly say that we are doing it for the above
reason (e.g. "In the next commit, we want to use the TempDir extension, but...")
|
Closed with changes made in #959 instead. |
LogicManagerTest and MainWindowCloseTest calls newFile() on a
temporary folder to initialize an empty file for the AddressBoook and
UserPrefs storage.
This is unnecessary, as the file is automatically created the first
time it is written to.
Instead of explicitly creating a new file, let's simply reference a
file path with Path#resolve instead.
Reference: #959 (comment)