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

Migrate file system tests to JUnit 4 #755

Merged
merged 2 commits into from
Oct 26, 2023

Conversation

HeikoKlare
Copy link
Contributor

The FileSystemTest and its subclasses are still based on JUnit 3 due to implementing the CoreTest class.
In addition the CoreTest class as a base class for many tests based on JUnit 3, provides several file system utilities to its subclasses. These utilities are, however, not inherent to the test class but can be provided as independent utilities. This is especially necessary to break up inheritance hierarchies and foster reuse for JUnit 4/5 migration.

This change does the following:

  • Moves functionalities concerning symlink creation and temp file creation to the FileSystemHelper
  • Migrates FileSystemTest and its subclasses to JUnit 4
  • Removes obsolete try/catch blocks by simply making the tests throw the exceptions or asserting the exceptions
  • Adds missing try-with-resources blocks for streams

@github-actions
Copy link
Contributor

github-actions bot commented Oct 20, 2023

Test Results

       42 files  ±0         42 suites  ±0   1h 7m 21s ⏱️ + 3m 11s
  3 778 tests ±0    3 775 ✔️ +1    3 💤 ±0  0  - 1 
11 337 runs  ±0  11 310 ✔️ +1  27 💤 ±0  0  - 1 

Results for commit 2553ae4. ± Comparison against base commit 9644844.

♻️ This comment has been updated with latest results.

The CoreTest class serves as a base class for many tests based on JUnit
3. It provides several file system utilities to its subclasses. These
utilities are, however, not inherent to the test class but can be
provided as independent utilities. This is especially necessary to break
up inheritance hierarchies and foster reuse for JUnit 4/5 migration.

This change moves functionalities concerning symlink creation and temp
file creation to the FileSystemHelper, which is supposed to provide such
functionality.
The FileSystemTest and its subclasses are still based on JUnit 3 due to
implementing the CoreTest class.

This change does the following:
* Migrates FileSystemTest and its subclasses to JUnit 4
* Removes obsolete try/catch blocks by simply making the tests throw the
exceptions or asserting the exceptions
* Adds missing try-with-resources blocks for streams
@HeikoKlare HeikoKlare force-pushed the filesystemtest-junit4 branch from de3ccbc to 2553ae4 Compare October 20, 2023 15:11
@HeikoKlare HeikoKlare marked this pull request as ready for review October 20, 2023 16:11
@akurtakov
Copy link
Member

Thanks!

@akurtakov akurtakov merged commit 826efd6 into eclipse-platform:master Oct 26, 2023
14 checks passed
@HeikoKlare HeikoKlare deleted the filesystemtest-junit4 branch October 26, 2023 17:35
Comment on lines +116 to +119
// The following code creates even a link if
// Files.createSymbolicLink(new File(basedir, linkName).toPath(), new
// File(basedir, linkTarget).toPath());
// would throw java.nio.file.FileSystemException "missing rights"
Copy link
Member

@HannesWell HannesWell Jun 22, 2024

Choose a reason for hiding this comment

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

@HeikoKlare can you tell if this is a common scenario (for tests)?
When looking at the code my fist thought was if Files.createSymbolicLink() can be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@HannesWell Unfortunately, I do not know whether this it is (still) a relevant/common scenario. I have only moved/refactored this code from the CoreTest class without challenging the necessity and way it is implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants