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 unit tests to JUnit 5 (Part 1) #471

Merged
merged 3 commits into from Jan 23, 2020

Conversation

KarboniteKream
Copy link
Member

This PR migrates part of unit tests to JUnit 5.

Changes:

  • Migrate some tests to JUnit 5
  • Use nested tests in ProjectServiceV1Test to reduce the number of Central Dogma instances
  • Upgrade Mockito to 3.2.4 and revert an earlier change in ZooKeeperCommandExecutorTest
  • Use assertThat().hasSize() where applicable

@codecov
Copy link

codecov bot commented Jan 19, 2020

Codecov Report

Merging #471 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #471      +/-   ##
============================================
- Coverage      69.9%   69.89%   -0.02%     
  Complexity     3149     3149              
============================================
  Files           325      325              
  Lines         12672    12674       +2     
  Branches       1348     1348              
============================================
  Hits           8858     8858              
- Misses         2963     2966       +3     
+ Partials        851      850       -1
Impacted Files Coverage Δ Complexity Δ
...r/internal/admin/auth/FileBasedSessionManager.java 76.25% <ø> (ø) 24 <0> (ø) ⬇️
.../centraldogma/testing/junit4/CentralDogmaRule.java 61.9% <0%> (-23.81%) 12% <0%> (-4%)
...centraldogma/server/internal/api/WatchService.java 75.86% <0%> (-3.45%) 12% <0%> (-1%)
...n/java/com/linecorp/centraldogma/common/Entry.java 93.87% <0%> (-2.05%) 29% <0%> (-1%)
...nternal/storage/repository/git/CommitWatchers.java 90.12% <0%> (-1.24%) 20% <0%> (-1%)
...rnal/storage/repository/git/PathPatternFilter.java 77.57% <0%> (-0.94%) 33% <0%> (-1%)
...necorp/centraldogma/testing/internal/TestUtil.java 81.81% <0%> (+4.04%) 4% <0%> (+2%) ⬆️
...server/internal/thrift/CentralDogmaExceptions.java 63.63% <0%> (+13.63%) 2% <0%> (ø) ⬇️
...traldogma/testing/junit/CentralDogmaExtension.java 84.21% <0%> (+17.54%) 23% <0%> (+6%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7152873...fd3cf7e. Read the comment docs.

Copy link
Member

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

🙇

List<Endpoint> decoded = decoder.decode(HOST_AND_PORT_LIST.stream().collect(Collectors.joining("\n")));
void decode() {
final EndpointListDecoder<String> decoder = EndpointListDecoder.TEXT;
final List<Endpoint> decoded = decoder.decode(String.join("\n", HOST_AND_PORT_LIST));
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -149,7 +148,7 @@ private static void verifyTwoIndependentCommands(Replica replica,
Command<?> command1,
Command<?> command2) {
final AtomicReference<Command<?>> lastCommand = new AtomicReference<>();
verify(replica.delegate, timeout(Duration.ofSeconds(2)).times(1)).apply(argThat(c -> {
verify(replica.delegate, timeout(TimeUnit.SECONDS.toMillis(2)).times(1)).apply(argThat(c -> {
Copy link
Contributor

@ikhoon ikhoon Jan 22, 2020

Choose a reason for hiding this comment

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

Nice 👍
I found timeout(Duration) has been reverted since 3.2.3 due to Android compatibility issue.
mockito/mockito#1843

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Big thanks to @KarboniteKream 🙇‍♂️

@trustin trustin merged commit 346a975 into line:master Jan 23, 2020
@trustin
Copy link
Member

trustin commented Jan 23, 2020

Thanks a lot and sorry I'm late!

@trustin trustin added this to the 0.43.5 milestone Jan 23, 2020
@KarboniteKream KarboniteKream deleted the junit/unit-tests-part1 branch January 23, 2020 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants