Mikhail Efremov created IGNITE-22388:
----------------------------------------
Summary: TableManagerTest revision
Key: IGNITE-22388
URL: https://issues.apache.org/jira/browse/IGNITE-22388
Project: Ignite
Issue Type: Improvement
Reporter: Mikhail Efremov
*Description*
During IGNITE-22315 we found that tests
{{TableManagerTable#tableManagerStopTest*}} are failed and after investigations
there was clear that several static mocks aren't in use in tests that leads to
actually unworked tests. The special case was fixed in IGNITE-22355, but there
are more issued places, especially described below. The main goal of the ticket
is to revise the test class and rewrite it in accordings to its purpose without
any excessive, unused code lines.
*Motivation*
There will be provided 3 examples with problematic code the was found in
{{TableManagerTable#tableManagerStopTest*}} but not limited.
The first code smell is the name of the tests:
{{TableManagerTable#tableManagerStopTest1-4}}. The names doesn't tell how 1st
and 4th are different.
The second is another (except that in IGNITE-22355) static mock
{{schemaServiceMock}} in try-with-resources block that shouldn't work outside
the block:
{code:title=|language=java|collapse=false}private TableViewInternal
mockManagersAndCreateTableWithDelay(
String tableName,
CompletableFuture<TableManager> tblManagerFut,
@Nullable Phaser phaser
) throws Exception {
String consistentId = "node0";
when(ts.getByConsistentId(any())).thenReturn(new ClusterNodeImpl(
UUID.randomUUID().toString(),
consistentId,
new NetworkAddress("localhost", 47500)
));
try (MockedStatic<SchemaUtils> schemaServiceMock =
mockStatic(SchemaUtils.class)) {
schemaServiceMock.when(() -> SchemaUtils.prepareSchemaDescriptor(any()))
.thenReturn(mock(SchemaDescriptor.class));
}
try (MockedStatic<AffinityUtils> affinityServiceMock =
mockStatic(AffinityUtils.class)) {
ArrayList<List<ClusterNode>> assignment = new ArrayList<>(PARTITIONS);
for (int part = 0; part < PARTITIONS; part++) {
assignment.add(new ArrayList<>(Collections.singleton(node)));
}
affinityServiceMock.when(() ->
AffinityUtils.calculateAssignments(any(), anyInt(), anyInt()))
.thenReturn(assignment);
}
//...
{code}
In the test class there are only two such mocks and {{affinityServiceMock}} is
removed in IGNITE-22355, but {{schemaServiceMock}} still requires reworking.
The third issue in the code above is {{String consistentId = "node0"}} for mock
{{when(ts.getByConsistentId(any()))}}. The problem is that {{ts}} isn't really
uses: {{ts}} is topology service field in the test class, but for
{{TableManager}} creation topology service arrives from a mocked cluster
service:
{code:title=|language=java|collapse=false}private TableManager
createTableManager(/*...*/) {
TableManager tableManager = new TableManager(
// ...
clusterService.topologyService(),
//...
{code}
{{topologyService}} is mocked in {{before}} with another mock:
{code:title=|language=java|collapse=false}@BeforeEach
void before() throws NodeStoppingException {
// ...
TopologyService topologyService = mock(TopologyService.class);
when(clusterService.topologyService()).thenReturn(topologyService);
// ...
{code}
That means, that {{ts}} field isn't in use and the code above is just for
nothing.
There only 3 arguments, but they shows drastical problems with the test class
and the ticket calls for the class reworking.
*Definition of done*
# Tests' names should have meaningful names that differs one from another.
# Tests shouldn't contain unused and meaningless code.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
