From f9178c5fea01e07d8301c8003af13fe9178002c5 Mon Sep 17 00:00:00 2001 From: Jeremy Bernard Date: Mon, 19 Feb 2024 18:38:25 +0100 Subject: [PATCH 1/3] Use `@DataMongoTest` and `@Testcontainers` annotations in `WorkerServiceTests` --- CHANGELOG.md | 2 +- .../java/com/iexec/core/worker/Worker.java | 18 +- .../iexec/core/metric/MetricServiceTests.java | 4 +- .../com/iexec/core/task/TaskServiceTests.java | 1 + .../WorkerServiceRealRepositoryTests.java | 192 ----------- .../iexec/core/worker/WorkerServiceTests.java | 302 ++++++++++++------ 6 files changed, 223 insertions(+), 296 deletions(-) delete mode 100644 src/test/java/com/iexec/core/worker/WorkerServiceRealRepositoryTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 036ca73b..394fbefa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ All notable changes to this project will be documented in this file. ### Quality -- Use `@DataMongoTest` and `@Testcontainers` annotations in replicates, compute logs and tasks tests. (#662 #664) +- Use `@DataMongoTest` and `@Testcontainers` annotations in replicates, compute logs and tasks tests. (#662 #664 #665) ## [[8.3.0]](https://github.com/iExecBlockchainComputing/iexec-core/releases/tag/v8.3.0) 2024-01-11 diff --git a/src/main/java/com/iexec/core/worker/Worker.java b/src/main/java/com/iexec/core/worker/Worker.java index eb5c9629..39ddc05f 100644 --- a/src/main/java/com/iexec/core/worker/Worker.java +++ b/src/main/java/com/iexec/core/worker/Worker.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 IEXEC BLOCKCHAIN TECH + * Copyright 2020-2024 IEXEC BLOCKCHAIN TECH * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,14 +23,13 @@ import org.springframework.data.mongodb.core.index.Indexed; import org.springframework.data.mongodb.core.mapping.Document; -import java.util.ArrayList; import java.util.Date; import java.util.List; -@Data @Document -@AllArgsConstructor +@Data @Builder +@AllArgsConstructor public class Worker { @Id @@ -47,18 +46,15 @@ public class Worker { private int memorySize; private boolean teeEnabled; private boolean gpuEnabled; - private List participatingChainTaskIds; - private List computingChainTaskIds; + @Builder.Default + private List participatingChainTaskIds = List.of(); + @Builder.Default + private List computingChainTaskIds = List.of(); // TODO remove it cleanly in a release private Date lastAliveDate; private Date lastReplicateDemandDate; - public Worker() { - participatingChainTaskIds = new ArrayList<>(); - computingChainTaskIds = new ArrayList<>(); - } - void addChainTaskId(String chainTaskId) { participatingChainTaskIds.add(chainTaskId); computingChainTaskIds.add(chainTaskId); diff --git a/src/test/java/com/iexec/core/metric/MetricServiceTests.java b/src/test/java/com/iexec/core/metric/MetricServiceTests.java index 79966819..c7f853c5 100644 --- a/src/test/java/com/iexec/core/metric/MetricServiceTests.java +++ b/src/test/java/com/iexec/core/metric/MetricServiceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 IEXEC BLOCKCHAIN TECH + * Copyright 2020-2024 IEXEC BLOCKCHAIN TECH * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -54,7 +54,7 @@ void init() { void shouldGetPlatformMetrics() { final LinkedHashMap expectedCurrentTaskStatusesCount = createExpectedCurrentTaskStatusesCount(); - List aliveWorkers = List.of(new Worker()); + List aliveWorkers = List.of(Worker.builder().build()); when(workerService.getAliveWorkers()).thenReturn(aliveWorkers); when(workerService.getAliveTotalCpu()).thenReturn(1); when(workerService.getAliveAvailableCpu()).thenReturn(1); diff --git a/src/test/java/com/iexec/core/task/TaskServiceTests.java b/src/test/java/com/iexec/core/task/TaskServiceTests.java index 91c325ac..9da1f19f 100644 --- a/src/test/java/com/iexec/core/task/TaskServiceTests.java +++ b/src/test/java/com/iexec/core/task/TaskServiceTests.java @@ -136,6 +136,7 @@ void shouldAddTask() { assertThat(saved) .usingRecursiveComparison() .ignoringFields("value.id", "value.version") + .ignoringFieldsOfTypes(List.class) .isEqualTo(Optional.of(task)); } diff --git a/src/test/java/com/iexec/core/worker/WorkerServiceRealRepositoryTests.java b/src/test/java/com/iexec/core/worker/WorkerServiceRealRepositoryTests.java deleted file mode 100644 index 821538b2..00000000 --- a/src/test/java/com/iexec/core/worker/WorkerServiceRealRepositoryTests.java +++ /dev/null @@ -1,192 +0,0 @@ -/* - * Copyright 2023-2023 IEXEC BLOCKCHAIN TECH - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.iexec.core.worker; - -import com.iexec.core.configuration.WorkerConfiguration; -import lombok.extern.slf4j.Slf4j; -import org.assertj.core.api.Assertions; -import org.awaitility.Awaitility; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import org.springframework.boot.test.autoconfigure.data.mongo.DataMongoTest; -import org.springframework.boot.test.mock.mockito.SpyBean; -import org.springframework.data.mongodb.core.MongoTemplate; -import org.springframework.test.context.DynamicPropertyRegistry; -import org.springframework.test.context.DynamicPropertySource; -import org.testcontainers.containers.MongoDBContainer; -import org.testcontainers.junit.jupiter.Container; -import org.testcontainers.junit.jupiter.Testcontainers; -import org.testcontainers.utility.DockerImageName; - -import java.time.Duration; -import java.util.*; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; -import java.util.stream.Collectors; -import java.util.stream.IntStream; - -import static com.iexec.commons.poco.utils.TestUtils.WALLET_WORKER_1; -import static org.assertj.core.api.Assertions.assertThat; - -@Slf4j -@DataMongoTest -@Testcontainers -class WorkerServiceRealRepositoryTests { - - private static final String WORKER_NAME = "worker1"; - private static final String WALLET_ADDRESS = "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"; - - @Container - private static final MongoDBContainer mongoDBContainer = new MongoDBContainer(DockerImageName.parse(System.getProperty("mongo.image"))); - - @DynamicPropertySource - static void registerProperties(DynamicPropertyRegistry registry) { - registry.add("spring.data.mongodb.host", mongoDBContainer::getHost); - registry.add("spring.data.mongodb.port", () -> mongoDBContainer.getMappedPort(27017)); - } - - @SpyBean - private MongoTemplate mongoTemplate; - @SpyBean - private WorkerRepository workerRepository; - @Mock - private WorkerConfiguration workerConfiguration; - private WorkerService workerService; - - private final Worker existingWorker = Worker.builder() - .id("1") - .name(WORKER_NAME) - .walletAddress(WALLET_ADDRESS) - .os("Linux") - .cpu("x86") - .cpuNb(8) - .participatingChainTaskIds(new ArrayList<>(Arrays.asList("task1", "task2"))) - .computingChainTaskIds(new ArrayList<>(Arrays.asList("task1", "task2"))) - .build(); - - @BeforeEach - void init() { - MockitoAnnotations.openMocks(this); - workerService = new WorkerService(mongoTemplate, workerRepository, workerConfiguration); - workerRepository.deleteAll(); - } - - /** - * Try and add N tasks to a single worker at the same time. - * If everything goes right, the Worker should finally have been assigned N tasks. - */ - @Test - void addMultipleTaskIds() { - final int nThreads = 10; - workerService.addWorker( - Worker.builder() - .walletAddress(WALLET_WORKER_1) - .maxNbTasks(nThreads) - .build() - ); - - final ExecutorService executor = Executors.newFixedThreadPool(nThreads); - - final List>> futures = IntStream.range(0, nThreads) - .mapToObj(i -> executor.submit(() -> workerService.addChainTaskIdToWorker(new Date().getTime() + "", WALLET_WORKER_1))) - .collect(Collectors.toList()); - - Awaitility.await() - .atMost(Duration.ofMinutes(1)) - .until(() -> futures.stream().map(Future::isDone).reduce(Boolean::logicalAnd).orElse(false)); - - Assertions.assertThat(workerService.getWorker(WALLET_WORKER_1).get().getComputingChainTaskIds()) - .hasSize(nThreads); - } - - // region removeChainTaskIdFromWorker - @Test - void shouldRemoveTaskIdFromWorker() { - workerRepository.save(existingWorker); - - Optional removedWorker = workerService.removeChainTaskIdFromWorker("task2", WALLET_ADDRESS); - assertThat(removedWorker).isPresent(); - Worker worker = removedWorker.get(); - assertThat(worker.getParticipatingChainTaskIds()).hasSize(1); - assertThat(worker.getParticipatingChainTaskIds().get(0)).isEqualTo("task1"); - assertThat(worker.getComputingChainTaskIds()).hasSize(1); - assertThat(worker.getComputingChainTaskIds().get(0)).isEqualTo("task1"); - } - - @Test - void shouldNotRemoveTaskIdWorkerNotFound() { - Optional addedWorker = workerService.removeChainTaskIdFromWorker("task1", "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"); - assertThat(addedWorker).isEmpty(); - } - - @Test - void shouldNotRemoveAnythingSinceTaskIdNotFound() { - workerRepository.save(existingWorker); - - Optional removedWorker = workerService.removeChainTaskIdFromWorker("dummyTaskId", WALLET_ADDRESS); - assertThat(removedWorker).isPresent(); - Worker worker = removedWorker.get(); - assertThat(worker.getParticipatingChainTaskIds()).hasSize(2); - assertThat(worker.getParticipatingChainTaskIds()).isEqualTo(List.of("task1", "task2")); - - assertThat(worker.getComputingChainTaskIds()).hasSize(2); - assertThat(worker.getComputingChainTaskIds()).isEqualTo(List.of("task1", "task2")); - } - // endregion - - // region removeComputedChainTaskIdFromWorker - @Test - void shouldRemoveComputedChainTaskIdFromWorker() { - workerRepository.save(existingWorker); - - Optional removedWorker = workerService.removeComputedChainTaskIdFromWorker("task1", WALLET_ADDRESS); - assertThat(removedWorker).isPresent(); - Worker worker = removedWorker.get(); - assertThat(worker.getParticipatingChainTaskIds()).hasSize(2); - assertThat(worker.getParticipatingChainTaskIds()).isEqualTo(List.of("task1", "task2")); - - assertThat(worker.getComputingChainTaskIds()).hasSize(1); - assertThat(worker.getComputingChainTaskIds().get(0)).isEqualTo("task2"); - } - - @Test - void shouldNotRemoveComputedChainTaskIdFromWorkerSinceWorkerNotFound() { - Optional removedWorker = workerService.removeComputedChainTaskIdFromWorker("task1", WALLET_ADDRESS); - assertThat(removedWorker).isEmpty(); - } - - @Test - void shouldNotRemoveComputedChainTaskIdFromWorkerSinceChainTaskIdNotFound() { - List participatingIds = List.of("task1", "task2"); - List computingIds = List.of("task1", "task2"); - - workerRepository.save(existingWorker); - - Optional removedWorker = workerService.removeComputedChainTaskIdFromWorker("dummyTaskId", WALLET_ADDRESS); - assertThat(removedWorker).isPresent(); - Worker worker = removedWorker.get(); - assertThat(worker.getParticipatingChainTaskIds()).hasSize(2); - assertThat(worker.getParticipatingChainTaskIds()).isEqualTo(participatingIds); - - assertThat(worker.getComputingChainTaskIds()).hasSize(2); - assertThat(worker.getComputingChainTaskIds()).isEqualTo(computingIds); - } - // endregion -} diff --git a/src/test/java/com/iexec/core/worker/WorkerServiceTests.java b/src/test/java/com/iexec/core/worker/WorkerServiceTests.java index afdbc0e1..8d90c2e0 100644 --- a/src/test/java/com/iexec/core/worker/WorkerServiceTests.java +++ b/src/test/java/com/iexec/core/worker/WorkerServiceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 IEXEC BLOCKCHAIN TECH + * Copyright 2020-2024 IEXEC BLOCKCHAIN TECH * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,41 +20,77 @@ import io.micrometer.core.instrument.Gauge; import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; -import org.assertj.core.api.Assertions; +import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.data.mongo.DataMongoTest; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.testcontainers.containers.MongoDBContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; +import org.testcontainers.utility.DockerImageName; import java.text.ParseException; -import java.text.SimpleDateFormat; +import java.time.Duration; import java.time.Instant; import java.util.*; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import static com.iexec.common.utils.DateTimeUtils.addMinutesToDate; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; +@DataMongoTest +@Testcontainers class WorkerServiceTests { - @Mock + private static final String WORKER1 = "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"; + private static final String WORKER2 = "0x2ab2674aa374fe6415d11f0a8fcbd8027fc1e6a9"; + + @Container + private static final MongoDBContainer mongoDBContainer = new MongoDBContainer(DockerImageName.parse(System.getProperty("mongo.image"))); + + @DynamicPropertySource + static void registerProperties(DynamicPropertyRegistry registry) { + registry.add("spring.data.mongodb.host", mongoDBContainer::getHost); + registry.add("spring.data.mongodb.port", () -> mongoDBContainer.getMappedPort(27017)); + } + + @Autowired private MongoTemplate mongoTemplate; - @Mock + @SpyBean + @Autowired private WorkerRepository workerRepository; @Mock private WorkerConfiguration workerConfiguration; - @InjectMocks private WorkerService workerService; + private final Worker existingWorker = Worker.builder() + .id("1") + .name("worker1") + .walletAddress(WORKER1) + .os("Linux") + .cpu("x86") + .cpuNb(8) + .participatingChainTaskIds(List.of("task1", "task2")) + .computingChainTaskIds(List.of("task1", "task2")) + .build(); @BeforeAll static void initRegistry() { @@ -64,6 +100,8 @@ static void initRegistry() { @BeforeEach void init() { MockitoAnnotations.openMocks(this); + workerService = new WorkerService(mongoTemplate, workerRepository, workerConfiguration); + workerRepository.deleteAll(); } @AfterEach @@ -80,32 +118,32 @@ void shouldReturnZeroForAllCountersWhereNothingHasAppended() { Gauge aliveTotalCpuGauge = Metrics.globalRegistry.find(WorkerService.METRIC_CPU_TOTAL_GAUGE).gauge(); Gauge aliveAvailableCpuGauge = Metrics.globalRegistry.find(WorkerService.METRIC_CPU_AVAILABLE_GAUGE).gauge(); - Assertions.assertThat(aliveWorkersGauge).isNotNull(); - Assertions.assertThat(aliveTotalCpuGauge).isNotNull(); - Assertions.assertThat(aliveAvailableCpuGauge).isNotNull(); - - Assertions.assertThat(aliveWorkersGauge.value()).isZero(); - Assertions.assertThat(aliveTotalCpuGauge.value()).isZero(); - Assertions.assertThat(aliveAvailableCpuGauge.value()).isZero(); + assertThat(aliveWorkersGauge).isNotNull(); + assertThat(aliveTotalCpuGauge).isNotNull(); + assertThat(aliveAvailableCpuGauge).isNotNull(); + assertThat(aliveWorkersGauge.value()).isZero(); + assertThat(aliveTotalCpuGauge.value()).isZero(); + assertThat(aliveAvailableCpuGauge.value()).isZero(); } @Test void shouldGetWorker() { String workerName = "worker1"; - String walletAddress = "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"; Worker existingWorker = Worker.builder() .id("1") .name(workerName) - .walletAddress(walletAddress) + .walletAddress(WORKER1) .os("Linux") .cpu("x86") .cpuNb(8) .build(); - when(workerRepository.findByWalletAddress(walletAddress)).thenReturn(Optional.of(existingWorker)); - Optional foundWorker = workerService.getWorker(walletAddress); - assertThat(foundWorker).contains(existingWorker); + workerRepository.save(existingWorker); + Optional foundWorker = workerService.getWorker(WORKER1); + assertThat(foundWorker) + .usingRecursiveComparison() + .isEqualTo(Optional.of(existingWorker)); } // addWorker @@ -131,8 +169,7 @@ void shouldNotAddNewWorker() { .cpuNb(8) .build(); - when(workerRepository.findByWalletAddress(walletAddress)).thenReturn(Optional.of(existingWorker)); - when(workerRepository.save(Mockito.any())).thenReturn(newWorker); + workerRepository.save(existingWorker); Worker addedWorker = workerService.addWorker(newWorker); assertThat(addedWorker).isNotEqualTo(existingWorker); @@ -142,22 +179,19 @@ void shouldNotAddNewWorker() { @Test void shouldAddNewWorker() { String workerName = "worker1"; - String walletAddress = "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"; Worker worker = Worker.builder() .name(workerName) - .walletAddress(walletAddress) + .walletAddress(WORKER1) .os("Linux") .cpu("x86") .cpuNb(8) .build(); - when(workerRepository.findByWalletAddress(walletAddress)).thenReturn(Optional.empty()); - when(workerRepository.save(Mockito.any())).thenReturn(worker); Worker addedWorker = workerService.addWorker(worker); // check that the save method was called once Mockito.verify(workerRepository, Mockito.times(1)).save(Mockito.any()); assertThat(addedWorker.getName()).isEqualTo(worker.getName()); - assertThat(workerService.getWorkerStatsMap().get(walletAddress).getLastAliveDate()).isBefore(new Date()); + assertThat(workerService.getWorkerStatsMap().get(WORKER1).getLastAliveDate()).isBefore(new Date()); } // isAllowedToJoin @@ -188,21 +222,19 @@ void shouldNotBeAllowed() { void shouldUpdateLastAlive() throws ParseException { // init String workerName = "worker1"; - String walletAddress = "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"; - Date oldLastAlive = new SimpleDateFormat("yyyy-MM-dd").parse("2018-01-01"); Worker worker = Worker.builder() .id("1") .name(workerName) - .walletAddress(walletAddress) + .walletAddress(WORKER1) .build(); - when(workerRepository.findByWalletAddress(walletAddress)).thenReturn(Optional.of(worker)); + workerRepository.save(worker); // call - workerService.updateLastAlive(walletAddress); + workerService.updateLastAlive(WORKER1); // check object returned by the method Date now = new Date(); - long duration = now.getTime() - workerService.getWorkerStatsMap().get(walletAddress).getLastAliveDate().getTime(); + long duration = now.getTime() - workerService.getWorkerStatsMap().get(WORKER1).getLastAliveDate().getTime(); long diffInSeconds = TimeUnit.MILLISECONDS.toSeconds(duration); assertThat(diffInSeconds).isZero(); } @@ -213,9 +245,10 @@ void shouldUpdateLastAlive() throws ParseException { void shouldWorkerBeAllowedToAskReplicate() { String wallet = "wallet"; Worker worker = Worker.builder() + .walletAddress(wallet) .build(); + workerRepository.save(worker); workerService.getWorkerStatsMap().computeIfAbsent(wallet, WorkerService.WorkerStats::new); - when(workerRepository.findByWalletAddress(wallet)).thenReturn(Optional.of(worker)); when(workerConfiguration.getAskForReplicatePeriod()).thenReturn(5000L); assertThat(workerService.isWorkerAllowedToAskReplicate(wallet)).isTrue(); @@ -255,16 +288,14 @@ void shouldUpdateLastReplicateDemand() { .isAfter(lastDate); } - // addChainTaskIdToWorker - + // region addChainTaskIdToWorker @Test void shouldAddTaskIdToWorker() { String workerName = "worker1"; - String walletAddress = "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"; Worker existingWorker = Worker.builder() .id("1") .name(workerName) - .walletAddress(walletAddress) + .walletAddress(WORKER1) .os("Linux") .cpu("x86") .cpuNb(8) @@ -273,10 +304,9 @@ void shouldAddTaskIdToWorker() { .computingChainTaskIds(new ArrayList<>(Arrays.asList("task1", "task2"))) .build(); - when(workerRepository.findByWalletAddress(walletAddress)).thenReturn(Optional.of(existingWorker)); - when(workerRepository.save(existingWorker)).thenReturn(existingWorker); + workerRepository.save(existingWorker); - Optional addedWorker = workerService.addChainTaskIdToWorker("task3", walletAddress); + Optional addedWorker = workerService.addChainTaskIdToWorker("task3", WORKER1); assertThat(addedWorker).isPresent(); Worker worker = addedWorker.get(); assertThat(worker.getParticipatingChainTaskIds()).hasSize(3); @@ -285,21 +315,47 @@ void shouldAddTaskIdToWorker() { assertThat(worker.getComputingChainTaskIds().get(2)).isEqualTo("task3"); } + /** + * Try and add N tasks to a single worker at the same time. + * If everything goes right, the Worker should finally have been assigned N tasks. + */ + @Test + void addMultipleTaskIds() { + final int nThreads = 10; + workerService.addWorker( + Worker.builder() + .walletAddress(WORKER1) + .maxNbTasks(nThreads) + .build() + ); + + final ExecutorService executor = Executors.newFixedThreadPool(nThreads); + + final List>> futures = IntStream.range(0, nThreads) + .mapToObj(i -> executor.submit(() -> workerService.addChainTaskIdToWorker(new Date().getTime() + "", WORKER1))) + .collect(Collectors.toList()); + + Awaitility.await() + .atMost(Duration.ofMinutes(1)) + .until(() -> futures.stream().map(Future::isDone).reduce(Boolean::logicalAnd).orElse(false)); + + assertThat(workerService.getWorker(WORKER1).get().getComputingChainTaskIds()) + .hasSize(nThreads); + } + @Test void shouldNotAddTaskIdToWorkerSinceUnknownWorker() { - when(workerRepository.findByWalletAddress(Mockito.anyString())).thenReturn(Optional.empty()); - Optional addedWorker = workerService.addChainTaskIdToWorker("task1", "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"); + Optional addedWorker = workerService.addChainTaskIdToWorker("task1", WORKER1); assertThat(addedWorker).isEmpty(); } @Test void shouldNotAddTaskIdToWorkerSinceCantAcceptMoreWorker() { String workerName = "worker1"; - String walletAddress = "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"; Worker existingWorker = Worker.builder() .id("1") .name(workerName) - .walletAddress(walletAddress) + .walletAddress(WORKER1) .os("Linux") .cpu("x86") .cpuNb(3) @@ -308,59 +364,115 @@ void shouldNotAddTaskIdToWorkerSinceCantAcceptMoreWorker() { .computingChainTaskIds(new ArrayList<>(Arrays.asList("task1", "task2"))) .build(); - when(workerRepository.findByWalletAddress(walletAddress)).thenReturn(Optional.of(existingWorker)); - when(workerRepository.save(existingWorker)).thenReturn(existingWorker); + workerRepository.save(existingWorker); - Optional addedWorker = workerService.addChainTaskIdToWorker("task3", walletAddress); + Optional addedWorker = workerService.addChainTaskIdToWorker("task3", WORKER1); assertThat(addedWorker).isEmpty(); } + // // getChainTaskIds @Test void shouldGetChainTaskIds() { - String wallet = "wallet"; List list = List.of("t1", "t1"); Worker worker = Worker.builder() + .walletAddress(WORKER1) .participatingChainTaskIds(list) .build(); - when(workerRepository.findByWalletAddress(wallet)).thenReturn(Optional.of(worker)); - - assertThat(workerService.getChainTaskIds(wallet)).isEqualTo(list); + workerRepository.save(worker); + assertThat(workerService.getChainTaskIds(WORKER1)).isEqualTo(list); } @Test void shouldGetEmptyChainTaskIdListSinceWorkerNotFound() { String wallet = "wallet"; - when(workerRepository.findByWalletAddress(wallet)).thenReturn(Optional.empty()); assertThat(workerService.getChainTaskIds(wallet)).isEmpty(); } - // Computing task IDs - + // region getComputingTaskIds @Test void shouldGetComputingTaskIds() { - String wallet = "wallet"; List list = List.of("t1", "t1"); Worker worker = Worker.builder() + .walletAddress(WORKER1) .computingChainTaskIds(list) .build(); - when(workerRepository.findByWalletAddress(wallet)).thenReturn(Optional.of(worker)); - - assertThat(workerService.getComputingTaskIds(wallet)).isEqualTo(list); + workerRepository.save(worker); + assertThat(workerService.getComputingTaskIds(WORKER1)).isEqualTo(list); } @Test void shouldNotGetComputingTaskIdsSinceNoWorker() { - String wallet = "wallet"; - when(workerRepository.findByWalletAddress(wallet)).thenReturn(Optional.empty()); + assertThat(workerService.getComputingTaskIds(WORKER2)).isEmpty(); + } + // endregion - assertThat(workerService.getComputingTaskIds(wallet)).isEmpty(); + // region removeChainTaskIdFromWorker + @Test + void shouldRemoveTaskIdFromWorker() { + workerRepository.save(existingWorker); + + final Worker worker = workerService.removeChainTaskIdFromWorker("task2", WORKER1).orElseThrow(); + assertThat(worker.getParticipatingChainTaskIds()).hasSize(1); + assertThat(worker.getParticipatingChainTaskIds().get(0)).isEqualTo("task1"); + assertThat(worker.getComputingChainTaskIds()).hasSize(1); + assertThat(worker.getComputingChainTaskIds().get(0)).isEqualTo("task1"); } @Test - void shouldGetLostWorkers() { + void shouldNotRemoveTaskIdWorkerNotFound() { + Optional addedWorker = workerService.removeChainTaskIdFromWorker("task1", "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"); + assertThat(addedWorker).isEmpty(); + } + + @Test + void shouldNotRemoveAnythingSinceTaskIdNotFound() { + workerRepository.save(existingWorker); + + final Worker worker = workerService.removeChainTaskIdFromWorker("dummyTaskId", WORKER1).orElseThrow(); + assertThat(worker.getParticipatingChainTaskIds()).hasSize(2); + assertThat(worker.getParticipatingChainTaskIds()).isEqualTo(List.of("task1", "task2")); + + assertThat(worker.getComputingChainTaskIds()).hasSize(2); + assertThat(worker.getComputingChainTaskIds()).isEqualTo(List.of("task1", "task2")); + } + // endregion + + // region removeComputedChainTaskIdFromWorker + @Test + void shouldRemoveComputedChainTaskIdFromWorker() { + workerRepository.save(existingWorker); + + final Worker worker = workerService.removeComputedChainTaskIdFromWorker("task1", WORKER1).orElseThrow(); + assertThat(worker.getParticipatingChainTaskIds()).hasSize(2); + assertThat(worker.getParticipatingChainTaskIds()).isEqualTo(List.of("task1", "task2")); + + assertThat(worker.getComputingChainTaskIds()).hasSize(1); + assertThat(worker.getComputingChainTaskIds().get(0)).isEqualTo("task2"); + } + + @Test + void shouldNotRemoveComputedChainTaskIdFromWorkerSinceWorkerNotFound() { + Optional removedWorker = workerService.removeComputedChainTaskIdFromWorker("task1", WORKER1); + assertThat(removedWorker).isEmpty(); + } + + @Test + void shouldNotRemoveComputedChainTaskIdFromWorkerSinceChainTaskIdNotFound() { + workerRepository.save(existingWorker); + final Worker worker = workerService.removeComputedChainTaskIdFromWorker("dummyTaskId", WORKER1).orElseThrow(); + assertThat(worker.getParticipatingChainTaskIds()).hasSize(2); + assertThat(worker.getParticipatingChainTaskIds()).isEqualTo(List.of("task1", "task2")); + + assertThat(worker.getComputingChainTaskIds()).hasSize(2); + assertThat(worker.getComputingChainTaskIds()).isEqualTo(List.of("task1", "task2")); + } + // endregion + + @Test + void shouldGetLostWorkers() { List allWorkers = getDummyWorkers(); List lostWorkers = allWorkers.subList(1, 3); @@ -381,10 +493,9 @@ void shouldNotFindLostWorkers() { @Test void shouldGetAliveWorkers() { - List allWorkers = getDummyWorkers(); List aliveWorkers = allWorkers.subList(0, 1); - when(workerRepository.findByWalletAddressIn(Mockito.any())).thenReturn(aliveWorkers); + workerRepository.saveAll(allWorkers); List claimedAliveWorkers = workerService.getAliveWorkers(); @@ -396,15 +507,12 @@ void shouldGetAliveWorkers() { @Test void shouldNotFindAliveWorkers() { - when(workerRepository.findByWalletAddressIn(Mockito.any())).thenReturn(Collections.emptyList()); assertThat(workerService.getAliveWorkers()).isEmpty(); } @Test void shouldAcceptMoreWorks() { - String walletAddress = "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"; - - Worker worker = getDummyWorker(walletAddress, + Worker worker = getDummyWorker(WORKER1, 3, Arrays.asList("task1", "task2", "task3", "task4", "task5"), Arrays.asList("task1", "task3")); @@ -414,9 +522,7 @@ void shouldAcceptMoreWorks() { @Test void shouldNotAcceptMoreWorksSinceSaturatedCpus() { - String walletAddress = "0x1a69b2eb604db8eba185df03ea4f5288dcbbd248"; - - Worker worker = getDummyWorker(walletAddress, + Worker worker = getDummyWorker(WORKER1, 2, Arrays.asList("task1", "task2", "task3", "task4"), Arrays.asList("task1", "task3")); @@ -449,17 +555,18 @@ Worker getDummyWorker(String walletAddress, int cpuNb, List participatin @Test void shouldGetSomeAvailableCpu() { - - Worker worker1 = getDummyWorker("0x1", + Worker worker1 = getDummyWorker(WORKER1, 4, Arrays.asList("task1", "task2", "task3", "task4"), Arrays.asList("task1", "task3"));//2 CPUs available - Worker worker2 = getDummyWorker("0x2", + Worker worker2 = getDummyWorker(WORKER2, 4, Arrays.asList("task1", "task2", "task3", "task4"), List.of("task1"));//3 CPUs available - when(workerRepository.findByWalletAddressIn(any())).thenReturn(Arrays.asList(worker1, worker2)); + workerRepository.saveAll(List.of(worker1, worker2)); + workerService.updateLastAlive(WORKER1); + workerService.updateLastAlive(WORKER2); assertThat(workerService.getAliveAvailableCpu()).isEqualTo(5); } @@ -467,24 +574,24 @@ void shouldGetSomeAvailableCpu() { @Test void shouldGetZeroAvailableCpuIfWorkerAlreadyFull() { - - Worker worker1 = getDummyWorker("0x1", + Worker worker1 = getDummyWorker(WORKER1, 4, Arrays.asList("task1", "task2", "task3", "task4"), Arrays.asList("task1", "task2", "task3", "task4")); - Worker worker2 = getDummyWorker("0x2", + Worker worker2 = getDummyWorker(WORKER2, 4, Arrays.asList("task1", "task2", "task3", "task4"), Arrays.asList("task1", "task2", "task3", "task4")); - when(workerRepository.findByWalletAddressIn(any())).thenReturn(Arrays.asList(worker1, worker2)); + workerRepository.saveAll(List.of(worker1, worker2)); + workerService.updateLastAlive(WORKER1); + workerService.updateLastAlive(WORKER2); assertThat(workerService.getAliveAvailableCpu()).isZero(); } @Test void shouldGetZeroAvailableCpuIfNoWorkerAlive() { - when(workerRepository.findByWalletAddressIn(any())).thenReturn(Collections.emptyList()); assertThat(workerService.getAliveAvailableCpu()).isZero(); } @@ -493,15 +600,19 @@ void shouldGetZeroAvailableCpuIfNoWorkerAlive() { @Test void shouldGetTotalAliveCpu() { Worker worker1 = Worker.builder() + .walletAddress(WORKER1) .cpuNb(4) .computingChainTaskIds(List.of("T1", "T2", "T3")) .build(); Worker worker2 = Worker.builder() + .walletAddress(WORKER2) .cpuNb(2) .computingChainTaskIds(List.of("T4")) .build(); List list = List.of(worker1, worker2); - when(workerRepository.findByWalletAddressIn(any())).thenReturn(list); + workerRepository.saveAll(list); + workerService.updateLastAlive(WORKER1); + workerService.updateLastAlive(WORKER2); workerService.init(); workerService.updateMetrics(); @@ -512,27 +623,34 @@ void shouldGetTotalAliveCpu() { Gauge aliveTotalCpuGauge = Metrics.globalRegistry.find(WorkerService.METRIC_CPU_TOTAL_GAUGE).gauge(); Gauge aliveAvailableCpuGauge = Metrics.globalRegistry.find(WorkerService.METRIC_CPU_AVAILABLE_GAUGE).gauge(); - Assertions.assertThat(aliveWorkersGauge).isNotNull(); - Assertions.assertThat(aliveTotalCpuGauge).isNotNull(); - Assertions.assertThat(aliveAvailableCpuGauge).isNotNull(); + assertThat(aliveWorkersGauge).isNotNull(); + assertThat(aliveTotalCpuGauge).isNotNull(); + assertThat(aliveAvailableCpuGauge).isNotNull(); - Assertions.assertThat(aliveWorkersGauge.value()).isEqualTo(list.size()); - Assertions.assertThat(aliveTotalCpuGauge.value()).isEqualTo(worker1.getCpuNb() + worker2.getCpuNb()); - Assertions.assertThat(aliveAvailableCpuGauge.value()).isEqualTo(2); + assertThat(aliveWorkersGauge.value()).isEqualTo(list.size()); + assertThat(aliveTotalCpuGauge.value()).isEqualTo(worker1.getCpuNb() + worker2.getCpuNb()); + assertThat(aliveAvailableCpuGauge.value()).isEqualTo(2); } // getAliveTotalGpu @Test void shouldGetTotalAliveGpu() { + final Date now = new Date(); Worker worker1 = Worker.builder() + .walletAddress(WORKER1) .gpuEnabled(true) + .lastAliveDate(now) .build(); Worker worker2 = Worker.builder() + .walletAddress(WORKER2) .gpuEnabled(false) + .lastAliveDate(now) .build(); List list = List.of(worker1, worker2); - when(workerRepository.findByWalletAddressIn(any())).thenReturn(list); + workerRepository.saveAll(list); + workerService.updateLastAlive(WORKER1); + workerService.updateLastAlive(WORKER2); assertThat(workerService.getAliveTotalGpu()).isEqualTo(1); } @@ -542,15 +660,19 @@ void shouldGetTotalAliveGpu() { @Test void shouldGetAliveAvailableGpu() { Worker worker1 = Worker.builder() + .walletAddress(WORKER1) .gpuEnabled(true) .computingChainTaskIds(List.of()) .build(); Worker worker2 = Worker.builder() + .walletAddress(WORKER2) .gpuEnabled(true) .computingChainTaskIds(List.of("t1")) .build(); List list = List.of(worker1, worker2); - when(workerRepository.findByWalletAddressIn(any())).thenReturn(list); + workerRepository.saveAll(list); + workerService.updateLastAlive(WORKER1); + workerService.updateLastAlive(WORKER2); assertThat(workerService.getAliveAvailableGpu()).isEqualTo(1); } From a8c46b3d0643a46b52cfe516824199d53fefbda4 Mon Sep 17 00:00:00 2001 From: Jeremy Bernard Date: Mon, 19 Feb 2024 19:30:34 +0100 Subject: [PATCH 2/3] Use regex to remove failling comparison --- src/test/java/com/iexec/core/task/TaskServiceTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/iexec/core/task/TaskServiceTests.java b/src/test/java/com/iexec/core/task/TaskServiceTests.java index 9da1f19f..d54b49a5 100644 --- a/src/test/java/com/iexec/core/task/TaskServiceTests.java +++ b/src/test/java/com/iexec/core/task/TaskServiceTests.java @@ -136,7 +136,7 @@ void shouldAddTask() { assertThat(saved) .usingRecursiveComparison() .ignoringFields("value.id", "value.version") - .ignoringFieldsOfTypes(List.class) + .ignoringFieldsMatchingRegexes("value.dateStatusList.*") .isEqualTo(Optional.of(task)); } From aeb00e18999cbc65e70840f72c5d24b8c1f58616 Mon Sep 17 00:00:00 2001 From: Jeremy Bernard Date: Tue, 20 Feb 2024 11:39:22 +0100 Subject: [PATCH 3/3] Fix tests after review --- .../com/iexec/core/worker/WorkerService.java | 2 +- .../iexec/core/worker/WorkerServiceTests.java | 45 +++++++++++-------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/iexec/core/worker/WorkerService.java b/src/main/java/com/iexec/core/worker/WorkerService.java index d98bbaf8..bdfdb294 100644 --- a/src/main/java/com/iexec/core/worker/WorkerService.java +++ b/src/main/java/com/iexec/core/worker/WorkerService.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 IEXEC BLOCKCHAIN TECH + * Copyright 2020-2024 IEXEC BLOCKCHAIN TECH * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/test/java/com/iexec/core/worker/WorkerServiceTests.java b/src/test/java/com/iexec/core/worker/WorkerServiceTests.java index 8d90c2e0..51a32672 100644 --- a/src/test/java/com/iexec/core/worker/WorkerServiceTests.java +++ b/src/test/java/com/iexec/core/worker/WorkerServiceTests.java @@ -26,11 +26,9 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.data.mongo.DataMongoTest; -import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.test.context.DynamicPropertyRegistry; import org.springframework.test.context.DynamicPropertySource; @@ -52,6 +50,7 @@ import static com.iexec.common.utils.DateTimeUtils.addMinutesToDate; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertAll; import static org.mockito.Mockito.when; @DataMongoTest @@ -72,7 +71,6 @@ static void registerProperties(DynamicPropertyRegistry registry) { @Autowired private MongoTemplate mongoTemplate; - @SpyBean @Autowired private WorkerRepository workerRepository; @@ -118,13 +116,17 @@ void shouldReturnZeroForAllCountersWhereNothingHasAppended() { Gauge aliveTotalCpuGauge = Metrics.globalRegistry.find(WorkerService.METRIC_CPU_TOTAL_GAUGE).gauge(); Gauge aliveAvailableCpuGauge = Metrics.globalRegistry.find(WorkerService.METRIC_CPU_AVAILABLE_GAUGE).gauge(); - assertThat(aliveWorkersGauge).isNotNull(); - assertThat(aliveTotalCpuGauge).isNotNull(); - assertThat(aliveAvailableCpuGauge).isNotNull(); - - assertThat(aliveWorkersGauge.value()).isZero(); - assertThat(aliveTotalCpuGauge.value()).isZero(); - assertThat(aliveAvailableCpuGauge.value()).isZero(); + assertAll( + () -> assertThat(aliveWorkersGauge) + .isNotNull() + .extracting(Gauge::value).isEqualTo(0.0), + () -> assertThat(aliveTotalCpuGauge) + .isNotNull() + .extracting(Gauge::value).isEqualTo(0.0), + () -> assertThat(aliveAvailableCpuGauge) + .isNotNull() + .extracting(Gauge::value).isEqualTo(0.0) + ); } @Test @@ -187,9 +189,10 @@ void shouldAddNewWorker() { .cpuNb(8) .build(); + assertThat(workerRepository.count()).isZero(); Worker addedWorker = workerService.addWorker(worker); // check that the save method was called once - Mockito.verify(workerRepository, Mockito.times(1)).save(Mockito.any()); + assertThat(workerRepository.count()).isOne(); assertThat(addedWorker.getName()).isEqualTo(worker.getName()); assertThat(workerService.getWorkerStatsMap().get(WORKER1).getLastAliveDate()).isBefore(new Date()); } @@ -476,7 +479,7 @@ void shouldGetLostWorkers() { List allWorkers = getDummyWorkers(); List lostWorkers = allWorkers.subList(1, 3); - when(workerRepository.findByWalletAddressIn(Mockito.any())).thenReturn(lostWorkers); + workerRepository.saveAll(allWorkers); List claimedLostWorkers = workerService.getLostWorkers(); @@ -623,13 +626,17 @@ void shouldGetTotalAliveCpu() { Gauge aliveTotalCpuGauge = Metrics.globalRegistry.find(WorkerService.METRIC_CPU_TOTAL_GAUGE).gauge(); Gauge aliveAvailableCpuGauge = Metrics.globalRegistry.find(WorkerService.METRIC_CPU_AVAILABLE_GAUGE).gauge(); - assertThat(aliveWorkersGauge).isNotNull(); - assertThat(aliveTotalCpuGauge).isNotNull(); - assertThat(aliveAvailableCpuGauge).isNotNull(); - - assertThat(aliveWorkersGauge.value()).isEqualTo(list.size()); - assertThat(aliveTotalCpuGauge.value()).isEqualTo(worker1.getCpuNb() + worker2.getCpuNb()); - assertThat(aliveAvailableCpuGauge.value()).isEqualTo(2); + assertAll( + () -> assertThat(aliveWorkersGauge) + .isNotNull() + .extracting(Gauge::value).isEqualTo((double) list.size()), + () -> assertThat(aliveTotalCpuGauge) + .isNotNull() + .extracting(Gauge::value).isEqualTo((double) worker1.getCpuNb() + worker2.getCpuNb()), + () -> assertThat(aliveAvailableCpuGauge) + .isNotNull() + .extracting(Gauge::value).isEqualTo(2.0) + ); } // getAliveTotalGpu