Skip to content

Conversation

@vogella
Copy link
Contributor

@vogella vogella commented Oct 17, 2025

No description provided.

@vogella
Copy link
Contributor Author

vogella commented Oct 17, 2025

I assume the test failure are related to

#2636 or #2641

@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2025

Test Results

 3 018 files  ±0   3 018 suites  ±0   2h 32m 10s ⏱️ - 2m 52s
 8 226 tests ±0   7 977 ✅ ±0  249 💤 ±0  0 ❌ ±0 
23 598 runs  ±0  22 804 ✅ ±0  794 💤 ±0  0 ❌ ±0 

Results for commit 7a2341d. ± Comparison against base commit 8ff6a17.

♻️ This comment has been updated with latest results.

@vogella
Copy link
Contributor Author

vogella commented Oct 18, 2025

@fedejeanne @akurtakov @HeikoKlare @HannesWell

Migration was done with Claude Code (which IMHO is currently the best AI engine for coding tasks). Changes look fine to me and the number of tests is still the same. Any concerns? Otherwise I start merging such changes if the build is green and the test number is the same (of course after manually checking for "strange" changes).

@HeikoKlare
Copy link
Contributor

In general if the changes stand up to manual review, I don't see any reason for not merging them.
I see some situations in which I would be more hesitant:

  • Everything related to session tests need a very thorough analysis and is probably better done manually as we have a completely different testing concept/framework for them with JUnit 5.
  • Whenever test class hierarchies are involved, as I have seen very few hierarchies that were really appropriate and most of them were better replaced by utilities, delegation, test rules/extensions etc.
  • Whenever test rules are involved, they need to be properly migrated to test extensions (or some other concept).

There might of course be different on opinions on how much cleanup such a migration should involve. This pretty much depends on the incentive or goal for doing a migration. For me, migrating to JUnit 5 was never (only) motivated by moving to a more modern framework, but I always took this as a chance to improve the test design and in particular test isolation. In particular in the Platform repository, test isolation was pretty bad before I migrated most of the stuff from JUnit 3/4 to JUnit 5 and the biggest win (and also my essential goal) were more stable tests due to improved isolation and not the migration to JUnit 5 itself.

@vogella
Copy link
Contributor Author

vogella commented Oct 18, 2025

Thanks @HeikoKlare for the detailed feedback. I agree with your goals and think AI can help with these too. In addition I see the following goals for me:

  • Using JUnit5 / JUnit6 should help with isolating and improving the tests in the future
  • Using modern test frameworks make the code base more interesting to new contributors
  • Migrating tests which can be easily migrated by AI tools makes the remaining problems easier to spot to us
  • Rule migration or test isolation should also be possible with AI tools

See also #3414 for some background analysis Claude did before starting to work on the task.

@vogella
Copy link
Contributor Author

vogella commented Oct 18, 2025

/gemini review

@vogella
Copy link
Contributor Author

vogella commented Oct 18, 2025

Change LGTM, planning to merge early next week

@vogella vogella merged commit c127f94 into eclipse-platform:master Oct 20, 2025
18 checks passed
@vogella vogella deleted the jface.text.junit5 branch October 20, 2025 08:06
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