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 Draw2D, GEF & Zest JUnit Tests from 3 to 4 #231

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

ptziegler
Copy link
Contributor

This change migrates all test cases to the latest JUnit version. With JUnit 4, annotations are used to control the execution flow. Given that the naming convention in JUnit 3 is rather fixed, a large chunk of this adaptation could be done automatically.

Normally, one would import statically import methods such as assertTrue(), assertEquals(), etc... However, those methods used to be provided by the TestCase class. In order to keep the code change to a minimum, we imitate this behavior by extending the Assert class instead.

The empty setUp() and tearDown() methods, as well as blank constructors have been removed.

Note that we only migrate to Junit4 instead of JUnit5, simply because Tycho is currently unable to execute those test suites. However, it should be much more straightforward to switch from 4 to 5 than from 3 to 4.

In short:

  • Extend Assert instead of TestCase
  • Annotate all tests with @test
  • Annotate disable tests/test classes with @ignore
  • Annotate setUp() method with @before, make public
  • Annotate afterEach() method with @after, make public
  • Convert test suites into JUnit4 format

This change migrates all test cases to the latest JUnit version. With
JUnit 4, annotations are used to control the execution flow. Given that
the naming convention in JUnit 3 is rather fixed, a large chunk of this
adaptation could be done automatically.

Normally, one would import statically import methods such as
assertTrue(), assertEquals(), etc... However, those methods used to be
provided by the TestCase class. In order to keep the code change to a
minimum, we imitate this behavior by extending the Assert class instead.

The empty setUp() and tearDown() methods, as well as blank
constructors have been removed.

Note that we only migrate to Junit4 instead of JUnit5, simply because
Tycho is currently unable to execute those test suites. However, it
should be much more straightforward to switch from 4 to 5 than from 3 to
4.

In short:
- Extend Assert instead of TestCase
- Annotate all tests with @test
- Annotate disable tests/test classes with @ignore
- Annotate setUp() method with @before, make public
- Annotate afterEach() method with @after, make public
- Convert test suites into JUnit4 format
@ptziegler
Copy link
Contributor Author

Note that both ColorConstantTest and TextFlowWrapTest are both currently not executed. I expect the same amount of successful tests (305) and 2 skipped tests.

@github-actions
Copy link

Unit Test Results

    9 files      9 suites   16s ⏱️
307 tests 305 ✔️ 2 💤 0
921 runs  915 ✔️ 6 💤 0

Results for commit 835b1a4.

@azoitl
Copy link
Contributor

azoitl commented Jul 31, 2023

Thx for this work this is great!

Regarding Junit 5, I wanted to point out that I know that in 4diac IDE we are using Tycho with Junit 5. If you are interested this is our root pom file: https://git.eclipse.org/c/4diac/org.eclipse.4diac.ide.git/tree/pom.xml?h=develop

@azoitl azoitl merged commit af29c5a into eclipse-gef:master Jul 31, 2023
4 checks passed
@ptziegler
Copy link
Contributor Author

I wanted to point out that I know that in 4diac IDE we are using Tycho with Junit 5

Hm... I seems like you're executing the tests directly, rather than going the extra step the test suites? That works without problems, but the moment you use suites, you run into this, i.e. none of the tests are executed.

In short, it seems like the Suite annotation isn't recognized by surefire. I have a suspicion what's causing this, but sadly not the time to look into this. 🙃

That aside, I think moving from JUnit4 to JUnit5 should be straightforward. My only concern is that they made changes to the test API. So instead of e.g. assertTrue(boolean, String), it's now assertTrue(String, boolean). Which is a real pain, because you can't run a simple string replacement on this.

@ptziegler ptziegler deleted the junit3-to-junit4 branch July 31, 2023 15:14
@azoitl
Copy link
Contributor

azoitl commented Jul 31, 2023

@ptziegler thx for the explanation. I must confess that I'm not the one who did your test infra. But explains some issues I had when upgrading dependencies.

@azoitl azoitl added this to the 3.17.0 milestone Jul 31, 2023
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