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 ConfigurationSessionTests in configuration package to JUnit 5 #654

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HeikoKlare
Copy link
Contributor

@HeikoKlare HeikoKlare commented Jul 5, 2024

The test classes EclipseStarterConfigurationAreaTest, MovableConfigurationAreaTest, and ReadOnlyConfigurationAreaTest rely on the JUnit-3-specific ConfigurationSessionTest. The SessionTestExtension has been introduced as a JUnit-5-based replacement.

This change adapts the test classes to be executed with JUnit 5, using the SessionTestExtension. It also adapts the according test suites to be executed with JUnit 5. I have only changed those test suites to JUnit 5 that are relevant for the updates tests. It would also be possible to just batch-migrate all test suite in a separate, preparatory PR. Let me know if that's desired.

This contribution serves two purposes:

  1. Modernization of the Equinox tests
  2. Modernization of the overall Eclipse Platform tests and getting rid of JUnit-3-based functionality (such as the ConfigurationSessionTest), see Migrate ResourceTests to JUnit 4/5 eclipse-platform/eclipse.platform#903

I will also provide PRs for updating the other ConfigurationSessionTest usages in Equinox once this update for the test classes in the configuration package has been approved.

Copy link

github-actions bot commented Jul 5, 2024

Test Results

  672 files  +12    672 suites  +12   1h 15m 15s ⏱️ -11s
2 194 tests  -  1  2 147 ✅  -  1   47 💤 ±0  0 ❌ ±0 
6 726 runs   -  3  6 583 ✅  -  3  143 💤 ±0  0 ❌ ±0 

Results for commit d21c556. ± Comparison against base commit f582f10.

This pull request removes 1180 and adds 1179 tests. Note that renamed tests count towards both.
AutomatedTests org.eclipse.equinox.log.test.AllTests org.eclipse.equinox.log.test.AllExtendedLogServiceTests org.eclipse.equinox.log.test.ExtendedLogReaderServiceTest ‑ testBadFilter
AutomatedTests org.eclipse.equinox.log.test.AllTests org.eclipse.equinox.log.test.AllExtendedLogServiceTests org.eclipse.equinox.log.test.ExtendedLogReaderServiceTest ‑ testExtendedLogEntry
AutomatedTests org.eclipse.equinox.log.test.AllTests org.eclipse.equinox.log.test.AllExtendedLogServiceTests org.eclipse.equinox.log.test.ExtendedLogReaderServiceTest ‑ testSynchronousLogListener
AutomatedTests org.eclipse.equinox.log.test.AllTests org.eclipse.equinox.log.test.AllExtendedLogServiceTests org.eclipse.equinox.log.test.ExtendedLogReaderServiceTest ‑ testaddFilteredListener
AutomatedTests org.eclipse.equinox.log.test.AllTests org.eclipse.equinox.log.test.AllExtendedLogServiceTests org.eclipse.equinox.log.test.ExtendedLogReaderServiceTest ‑ testaddFilteredListenerTwice
AutomatedTests org.eclipse.equinox.log.test.AllTests org.eclipse.equinox.log.test.AllExtendedLogServiceTests org.eclipse.equinox.log.test.ExtendedLogReaderServiceTest ‑ testaddNullFilterr
AutomatedTests org.eclipse.equinox.log.test.AllTests org.eclipse.equinox.log.test.AllExtendedLogServiceTests org.eclipse.equinox.log.test.ExtendedLogReaderServiceTest ‑ testaddNullListener
AutomatedTests org.eclipse.equinox.log.test.AllTests org.eclipse.equinox.log.test.AllExtendedLogServiceTests org.eclipse.equinox.log.test.ExtendedLogServiceTest ‑ testIsLoggableTrue
AutomatedTests org.eclipse.equinox.log.test.AllTests org.eclipse.equinox.log.test.AllExtendedLogServiceTests org.eclipse.equinox.log.test.ExtendedLogServiceTest ‑ testLogContext
AutomatedTests org.eclipse.equinox.log.test.AllTests org.eclipse.equinox.log.test.AllExtendedLogServiceTests org.eclipse.equinox.log.test.ExtendedLogServiceTest ‑ testLogContextWithNullThrowable
…
AutomatedTests AllFrameworkHookTests org.eclipse.osgi.tests.hooks.framework.ActivatorOrderTest ‑ testActivatorOrder
AutomatedTests AllFrameworkHookTests org.eclipse.osgi.tests.hooks.framework.BundleFileWrapperFactoryHookTests ‑ testGetResourceURL
AutomatedTests AllFrameworkHookTests org.eclipse.osgi.tests.hooks.framework.ClassLoaderHookTests ‑ testFilterClassPaths
AutomatedTests AllFrameworkHookTests org.eclipse.osgi.tests.hooks.framework.ClassLoaderHookTests ‑ testPreventResourceLoadFromClassLoadingHook
AutomatedTests AllFrameworkHookTests org.eclipse.osgi.tests.hooks.framework.ClassLoaderHookTests ‑ testRecursionFromClassLoadingHookIsSupported
AutomatedTests AllFrameworkHookTests org.eclipse.osgi.tests.hooks.framework.ClassLoaderHookTests ‑ testRecursionFromClassLoadingHookNotSupported
AutomatedTests AllFrameworkHookTests org.eclipse.osgi.tests.hooks.framework.ClassLoaderHookTests ‑ testRejectTransformationFromClassLoadingHook
AutomatedTests AllFrameworkHookTests org.eclipse.osgi.tests.hooks.framework.ClassLoaderHookTests ‑ testRejectTransformationFromWeavingHook
AutomatedTests AllFrameworkHookTests org.eclipse.osgi.tests.hooks.framework.ContextFinderTests ‑ testContextClassLoaderNullLocal
AutomatedTests AllFrameworkHookTests org.eclipse.osgi.tests.hooks.framework.DevClassPathDuplicateTests ‑ testDevClassPathWithExtension
…
This pull request removes 4 skipped tests and adds 4 skipped tests. Note that renamed tests count towards both.
AutomatedTests org.eclipse.osgi.tests.bundles.BundleTests org.eclipse.osgi.tests.bundles.ClassLoadingBundleTests ‑ testXFriends
AutomatedTests org.eclipse.osgi.tests.bundles.BundleTests org.eclipse.osgi.tests.bundles.PlatformAdminBundleTests ‑ testUnresolvedLeaves01
AutomatedTests org.eclipse.osgi.tests.bundles.BundleTests org.eclipse.osgi.tests.bundles.SystemBundleTests ‑ tTestBug351519RefreshEnabled
AutomatedTests org.eclipse.osgi.tests.bundles.BundleTests org.eclipse.osgi.tests.bundles.SystemBundleTests ‑ testMRUBundleFileListExpectedToFail
AutomatedTests BundleTests org.eclipse.osgi.tests.bundles.ClassLoadingBundleTests ‑ testXFriends
AutomatedTests BundleTests org.eclipse.osgi.tests.bundles.PlatformAdminBundleTests ‑ testUnresolvedLeaves01
AutomatedTests BundleTests org.eclipse.osgi.tests.bundles.SystemBundleTests ‑ tTestBug351519RefreshEnabled
AutomatedTests BundleTests org.eclipse.osgi.tests.bundles.SystemBundleTests ‑ testMRUBundleFileListExpectedToFail

♻️ This comment has been updated with latest results.

@HeikoKlare HeikoKlare force-pushed the configurationsessiontests-configuration-junit5 branch from ecd2ae6 to f92323f Compare July 5, 2024 09:21
The test classes EclipseStarterConfigurationAreaTest,
MovableConfigurationAreaTest, and ReadOnlyConfigurationAreaTest rely on
the JUnit-3-specific ConfigurationSessionTest. The SessionTestExtension
has been introduced as a JUnit-5-based replacement.
This change adapts the test classes to be executed with JUnit 5, using
the SessionTestExtension. It also adapts the according test suites to be
executed with JUnit 5.
@HeikoKlare HeikoKlare force-pushed the configurationsessiontests-configuration-junit5 branch from f92323f to d21c556 Compare July 5, 2024 10:16
@HeikoKlare HeikoKlare marked this pull request as ready for review July 5, 2024 13:43
@HannesWell
Copy link
Member

HannesWell commented Jul 7, 2024

Thanks Heiko for this.
I'll try to have a detailed look as soon as possible (maybe already tomorrow).
Just one first question: Is a deterministic order of execution for the test cases annotated with @Order strictly necessary or is this just nice to have? In case it's the latter I would suggest to remove that.

Edit: After looking at it again, the order is indeed necessary.

@HeikoKlare
Copy link
Contributor Author

Thank you, Hannes!
In short: yes, the order annotations are (usually) necessary. The existing ConfigurationSessionTest/ConfigurationSessionTestSuite executes test methods ordered by their names by default. First, I find this slightly confusing, as you have to know about how that test suite behaves to understand and define a proper execution order. Second, the new JUnit 5 SessionTestExtension only hooks into the execution of a single test case rather than orchestrating the whole test class, which makes it impossible to enforce an execution order. There would, of course, have been other means than a JUnit 5 extension to realize session tests, but I found the current solution of having the configurable SessionTestExtension together with the possibility to define for each test case whether it is executed in a separate session (default) or in the host (via @0ExecuteInHost) to be most appropriate.
As an additional information: I have already adapted all the Equinox session tests while introducing the SessionTestExtension to the Eclipse Platform to be quite sure that it will work properly. I just wanted to start with a smaller set of tests in this PR as a starting point for potential initial discussions on the solution.

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.

2 participants