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.

Sign up for GitHub

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

Jump to bottom

Migrate the non-UI tests from JUnit 4 to 5 #1456

Merged
merged 23 commits into from
Jan 28, 2023
Merged

Migrate the non-UI tests from JUnit 4 to 5 #1456

merged 23 commits into from
Jan 28, 2023

Conversation

mkondratek
Copy link
Collaborator

No description provided.

@mkondratek mkondratek self-assigned this Jan 17, 2023
@mkondratek mkondratek linked an issue Jan 17, 2023 that may be closed by this pull request
Migrate the non-UI tests from JUnit 4 to 5 #1365
Closed
mkondratek
mkondratek commented Jan 17, 2023
View reviewed changes
src/uiTest/scala/com/virtuslab/gitmachete/uitest/UIScenarioSuite.scala
@@ -10,7 +10,7 @@ import org.virtuslab.ideprobe.ProbeDriver
object UIScenarioSuite extends UISuite {}

@RunWith(classOf[JUnit4])
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

here the stairs are starting

@PawelLipski, is it true that all non ui tests are now migrated to junit 5?
If so, maybe let's try to get it merged and I will try to deliver junit 5 in ui tests later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, looks like that indeed 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've extracted VirtusLab/ide-probe#337 and #1464 for that

Copy link
Collaborator

Choose a reason for hiding this comment

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

And narrowed down #1365 to non-UI tests explicitly

@mkondratek mkondratek marked this pull request as ready for review January 17, 2023 16:19
PawelLipski
PawelLipski reviewed Jan 17, 2023
View reviewed changes
.../com/virtuslab/gitmachete/testcommon/GitRepositoryBackedIntegrationTestSuiteInitializer.java Outdated
@@ -8,16 +8,16 @@

import lombok.SneakyThrows;

public abstract class BaseGitRepositoryBackedIntegrationTestSuite {
public class GitRepositoryBackedIntegrationTestSuiteInitializer {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it the final form of this class? or has sth been retained just for the sake of supporting junit4 in UI tests?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also... I'd simplify the name of this class. I'd argue that it theoretically doesn't need to be used to "back" any "integration test suite". It really just wraps a git repo prepared from the given script in a temp directory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

before parameters were used to construct the test classes - I think that the parameters functionality was overused there. JUnit 5 more strictly requires that parameters are for test methods (cases).

It is still needed for ui tests - that's true. but I think some improvements (esp naming) can be applied even though ui tests leave with junit 4 for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Renamed to just TestGitRepository

@PawelLipski PawelLipski added testing Relates to tests and testability code quality Code quality labels Jan 19, 2023
@PawelLipski PawelLipski added this to the v3.4.0 milestone Jan 19, 2023
PawelLipski
PawelLipski previously approved these changes Jan 19, 2023
View reviewed changes
@mkondratek mkondratek force-pushed the chore/junit-5 branch 2 times, most recently from c1922e9 to fbe7b10 Compare January 20, 2023 12:28
@mkondratek mkondratek force-pushed the feature/rename branch 2 times, most recently from 3e4d7d0 to 65634b4 Compare January 20, 2023 12:55
@PawelLipski
Copy link
Collaborator

Hmmm actually this doesn't need to depend on rename feature, can go directly to develop... lemme take it over & retarget. Later I'll reconcile these changes with #1460 which adds a non-trivial use of PowerMock

@PawelLipski PawelLipski marked this pull request as ready for review January 27, 2023 16:07
@PawelLipski PawelLipski force-pushed the refactor/arch-unit-prod-classes branch from ea4b9f4 to 4f09e02 Compare January 28, 2023 16:04
mkondratek and others added 22 commits January 28, 2023 17:04
@mkondratek @PawelLipski
Migrate to JUnit 5 (closes #1365)
1cc1aed 
- buildSrc
@mkondratek @PawelLipski
Migrate to JUnit 5 (closes #1365)
4f37607 
- branchLayout
@mkondratek @PawelLipski
Migrate to JUnit 5 (closes #1365)
a4da6ed 
- gitCore
@mkondratek @PawelLipski
Migrate to JUnit 5 (closes #1365)
5f5aaf1 
- frontend:ui
@mkondratek @PawelLipski
Migrate to JUnit 5 (closes #1365)
8babe28 
- frontend:resourcebundles
@mkondratek @PawelLipski
Migrate to JUnit 5 (closes #1365)
e96ace1 
- testCommon
@mkondratek @PawelLipski
Migrate to JUnit 5 (closes #1365)
29d0002 
- backend
@mkondratek @PawelLipski
Migrate to JUnit 5 (closes #1365)
da077b2 
- uiTest (modified but not migrated yet)
@mkondratek @PawelLipski
Migrate to JUnit 5 (closes #1365)
a3610e0 
- drop junit 4
@PawelLipski
Migrate to JUnit 5 (closes #1365) - actually enable the tests
93b4b18
@mkondratek @PawelLipski
Migrate to JUnit 5 (closes #1365)
3c94c07 
- revamp GitCoreRepositoryTest into integration test
@PawelLipski
Tidy up gitCore:jgit
6cc766c
@PawelLipski
Tidy up frontend:ui:impl
3227fc8
@PawelLipski
Tidy up buildSrc
2263260
@PawelLipski
Tidy up backend:impl
303f5af
@PawelLipski
Tidy up top-level project
13c1642
@PawelLipski
Tidy up testCommon
526d45d
@PawelLipski
Extract junitPlatformLauncher()
bd443c7
@PawelLipski
Tidy up :test
85c1569
@PawelLipski
Tidy up :test - Kotlin
7fd7f5a
@PawelLipski
Restore UI tests
40ba1eb
@PawelLipski
Mark GitMacheteRepositoryCache(IDependencyResolver) constructor as …
60c05a2 
…`@NonInjectable`
Base automatically changed from refactor/arch-unit-prod-classes to develop January 28, 2023 16:35
mkondratek
mkondratek commented Jan 28, 2023
View reviewed changes
Copy link
Collaborator Author

@mkondratek mkondratek left a comment

Choose a reason for hiding this comment

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

approving my own PR ✅ next time I find a use case for power mock I reconsider my code first XD

on the other hand - extracting some classes (submitter info, aux), think a bit, and it can be dropped !

branchLayout/api/src/test/java/com/virtuslab/branchlayout/api/BranchLayoutTestSuite.java Show resolved Hide resolved
...ab/gitmachete/backend/unit/GitMacheteRepository_deriveParentAwareForkPointUnitTestSuite.java Show resolved Hide resolved
...ab/gitmachete/backend/unit/GitMacheteRepository_deriveParentAwareForkPointUnitTestSuite.java Show resolved Hide resolved
...re/jGit/src/test/java/com/virtuslab/gitcore/impl/jgit/integration/GitCoreRepositoryTest.java

val gitCoreRepository = new GitCoreRepository(it.rootDirectoryPath, it.mainGitDirectoryPath, it.worktreeGitDirectoryPath);
// Let's check against a non-existent commit.
// No exception should be thrown, just a null returned.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

można? można!

@PawelLipski PawelLipski merged commit 09c022d into develop Jan 28, 2023
@PawelLipski PawelLipski deleted the chore/junit-5 branch January 28, 2023 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PawelLipski PawelLipski PawelLipski left review comments

Assignees

@PawelLipski PawelLipski

@mkondratek mkondratek

Labels
code quality Code quality testing Relates to tests and testability
Projects
None yet
Milestone
v3.5.0
Development

Successfully merging this pull request may close these issues.

Migrate the non-UI tests from JUnit 4 to 5
2 participants
@mkondratek @PawelLipski