[
https://issues.apache.org/jira/browse/IGNITE-22388?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ivan Zlenko updated IGNITE-22388:
---------------------------------
Fix Version/s: 3.1
> Recheck tests from TableManagerTest for correctness
> ----------------------------------------------------
>
> Key: IGNITE-22388
> URL: https://issues.apache.org/jira/browse/IGNITE-22388
> Project: Ignite
> Issue Type: Improvement
> Reporter: Mikhail Efremov
> Assignee: Ivan Zlenko
> Priority: Major
> Labels: ignite-3
> Fix For: 3.1
>
>
> *Description*
> During IGNITE-22315 we found that tests
> {{TableManagerTest#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:java}
> 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: 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:java}
> private TableManager createTableManager(/*...*/) {
> TableManager tableManager = new TableManager(
> // ...
> clusterService.topologyService(),
> //...
> {code}
> {{topologyService}} is mocked in {{before}} with another mock:
> {code:java}
> @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)
