chore: upgrade gradle,java and spring-boot #546
Conversation
jonas-grgt
force-pushed
the
upgrade
branch
3 times, most recently
from
e050498
to
d1b4a60
Compare
|
Thanks. |
There was a problem hiding this comment.
Thanks. Well there are some tests failures that I can't explain I was hoping someone could help me with or give me some pointers. The failing tests run fine in isolation only together they fail I believe it must be related to not properly cleaning up the previous state but having a hard time proving it.
Thanks for the updates @jonasgeiregat . Do you mind pointing me to exact tests and I will give them a 👓 (I am guessing it has something to do w/ the Powermock (static mocking removals).
|
To make it clear the goal was not to get things merged in but rather to have something to work together on as moving this project forward is quite a big and challenging task. View it as a collaboration branch. |
|
I don't believe the failing tests have anything to do with the removal of powermock but I might be wrong. As I see it it has something to do with database state not being properly cleaned after a test run. For example the following test passes if I take in to account previous state (see comments) @ActiveProfiles("test")
public class TenantsRepositoryImplTest {
@Test
public void getTenantsListTest() {
Page<TenantEntity> tenantsEntities = tenantsRepository.getTenantsList(1, 10);
long existingTotal = tenantsEntities.getTotal(); // I added this and the above line of code to let the test pass
for (int i = 0; i < 10; i++) {
TenantEntity tenantEntity = new TenantEntity();
tenantEntity.setTenant("test" + i);
tenantEntity.setAdminRoles("test" + i);
tenantEntity.setAllowedClusters("test-cluster");
tenantEntity.setEnvironmentName("test-environment");
tenantsRepository.save(tenantEntity);
}
tenantsEntities = tenantsRepository.getTenantsList(1, 10);
tenantsEntities.count(true);
long total = tenantsEntities.getTotal();
Assert.assertEquals(10 + existingTotal, total); // by adding this to the assert the test passes reliablyThe main test that fails is |
|
Good work @jonasgeiregat! There's a merge conflict in src/test/java/org/apache/pulsar/manager/service/BrokerTokensServiceImplTest.java . Please merge changes from master branch to the PR and resolve the conflict. |
Bumped gradle to 8.5 Bumped spring-boot to 2.5.3 Removed useless BookiesServiceImplTest that was not asserting anything. Removed static nature of HttpUtils to make it injectable and better testable (favor Mockito over staticly mocking things using Powermock).
Turned out newer versions of page helper do not work zero indexed but rather use one as the first page.
Removed unnecessary mocking of httpUtil. The id generation is not always deterministic when saving an entity. Therefore, I opted to let the test code use the id returned by the save method instead of relying on a deterministic value. Enabled distTar again by replacing the deprecated property by the new one.
Fixed the merge conflicts |
|
Great work @jonasgeiregat , merged |
|
@jonasgeiregat do you have a chance to bump the Pulsar version? The current 2.7.0 is very old. We could go with either LTS 3.0.2 or most recent 3.2.0 version. |
|
Will take a look at it |
|
It seems that the log4j version is pinned: Line 102 in 0c87834 |
No description provided.