You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/03/11 09:31:25 UTC
[GitHub] [fineract] galovics commented on a change in pull request #2137: FINERACT-1508: Add Cucumber test suppport
galovics commented on a change in pull request #2137:
URL: https://github.com/apache/fineract/pull/2137#discussion_r824505873
##########
File path: fineract-provider/src/test/java/org/apache/fineract/TestSuite.java
##########
@@ -16,23 +16,19 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.fineract.infrastructure.core.boot.tests;
+package org.apache.fineract;
-import org.apache.fineract.ServerWithMariaDB4jApplication;
+import io.cucumber.spring.CucumberContextConfiguration;
import org.junit.jupiter.api.extension.ExtendWith;
-import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.TestPropertySource;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.context.web.WebAppConfiguration;
+@CucumberContextConfiguration
@ExtendWith(SpringExtension.class)
-@SpringBootTest(classes = ServerWithMariaDB4jApplication.Configuration.class)
@TestPropertySource("classpath:application-test.properties")
@WebAppConfiguration
-public abstract class AbstractSpringBootWithMariaDB4jIntegrationTest {
-
- // do NOT put any helper methods here!
- // it's much better to use composition instead of inheritance
- // so write a test util ("fixture") and use it as a field in your test
-
-}
+@ContextConfiguration(classes = TestConfiguration.class)
+// @SpringBootTest(classes = TestConfiguration.class)
Review comment:
I guess we can remvoe this commented line.
##########
File path: fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/LiquibaseDisabledScenario.java
##########
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.fineract.infrastructure.core;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.BDDMockito.given;
+
+import com.zaxxer.hikari.HikariDataSource;
+import io.cucumber.java8.En;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.service.migration.ExtendedSpringLiquibaseFactory;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDataSourceFactory;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseStateVerifier;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseUpgradeService;
+import org.apache.fineract.infrastructure.security.service.TenantDetailsService;
+import org.springframework.beans.factory.annotation.Autowired;
+
+public class LiquibaseDisabledScenario implements En {
Review comment:
Probably it would make sense to put all liquibase related steps into this file to keep it consistent.
##########
File path: fineract-provider/src/test/java/org/apache/fineract/batch/builder/BatchBuilderScenario.java
##########
@@ -0,0 +1,62 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.fineract.batch.builder;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import io.cucumber.java8.En;
+import java.util.Collections;
+import java.util.List;
+import javax.ws.rs.core.UriInfo;
+import org.apache.fineract.batch.domain.BatchRequest;
+import org.apache.fineract.batch.domain.BatchResponse;
+import org.apache.fineract.batch.service.BatchApiService;
+
+public class BatchBuilderScenario implements En {
+
+ private BatchApiService batchApiService;
+
+ private List<BatchRequest> requests;
+
+ private UriInfo uriInfo;
+
+ private List<BatchResponse> responses;
+
+ public BatchBuilderScenario() {
+ Given("A batch request", () -> {
+ this.batchApiService = mock(BatchApiService.class);
+
+ this.uriInfo = mock(UriInfo.class);
+
+ this.requests = Collections.singletonList(new BatchRequest());
+ });
+
+ When("The user calls the batch service handle request method", () -> {
Review comment:
I think it would be important to highlight in the step definition that this batch API will not care about the enclosing transaction. We could call it like:
`The user calls the batch service handle request method without enclosing transaction`
What do you think?
Preferably, we should have a test for verifying with and without the enclosing transaction behavior works properly. Right now I think we don't have tests for the with enclosing transaction way and I'm not saying you should introduce it, but for the future, we'll definitely create some.
##########
File path: fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/LiquibaseDisabledScenario.java
##########
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.fineract.infrastructure.core;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.BDDMockito.given;
+
+import com.zaxxer.hikari.HikariDataSource;
+import io.cucumber.java8.En;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.service.migration.ExtendedSpringLiquibaseFactory;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDataSourceFactory;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseStateVerifier;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseUpgradeService;
+import org.apache.fineract.infrastructure.security.service.TenantDetailsService;
+import org.springframework.beans.factory.annotation.Autowired;
+
+public class LiquibaseDisabledScenario implements En {
+
+ @Autowired
+ private TenantDetailsService tenantDetailsService;
+
+ @Autowired
+ private HikariDataSource tenantDataSource;
+
+ @Autowired
+ private FineractProperties fineractProperties;
+
+ @Autowired
+ private TenantDatabaseStateVerifier databaseStateVerifier;
+
+ @Autowired
+ private ExtendedSpringLiquibaseFactory liquibaseFactory;
+
+ @Autowired
+ private TenantDataSourceFactory tenantDataSourceFactory;
+
+ @Autowired
+ private TenantDatabaseUpgradeService underTest;
+
+ public LiquibaseDisabledScenario() {
+ Given("Liquibase is disabled", () -> {
+ given(databaseStateVerifier.isLiquibaseDisabled()).willReturn(true);
+ });
+
+ When("After properties set is executed", () -> {
+ underTest.afterPropertiesSet();
+ });
+
+ Then("The Liquibase configuration is unchanged", () -> {
+ assertTrue(databaseStateVerifier.isLiquibaseDisabled());
Review comment:
Well, the verifications were meant to make sure that nothing gets interacted with in case Liquibase is disabled, like there should be no interaction with figuring out the current database state, preparing the Liquibase migrations, nothing.
The assertion here will always pass since it's a mock and has been initialized with a mock value of true.
##########
File path: fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/LiquibaseDisabledScenario.java
##########
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.fineract.infrastructure.core;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.BDDMockito.given;
+
+import com.zaxxer.hikari.HikariDataSource;
+import io.cucumber.java8.En;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.service.migration.ExtendedSpringLiquibaseFactory;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDataSourceFactory;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseStateVerifier;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseUpgradeService;
+import org.apache.fineract.infrastructure.security.service.TenantDetailsService;
+import org.springframework.beans.factory.annotation.Autowired;
+
+public class LiquibaseDisabledScenario implements En {
+
+ @Autowired
+ private TenantDetailsService tenantDetailsService;
+
+ @Autowired
+ private HikariDataSource tenantDataSource;
+
+ @Autowired
+ private FineractProperties fineractProperties;
+
+ @Autowired
+ private TenantDatabaseStateVerifier databaseStateVerifier;
+
+ @Autowired
+ private ExtendedSpringLiquibaseFactory liquibaseFactory;
+
+ @Autowired
+ private TenantDataSourceFactory tenantDataSourceFactory;
+
+ @Autowired
+ private TenantDatabaseUpgradeService underTest;
+
+ public LiquibaseDisabledScenario() {
+ Given("Liquibase is disabled", () -> {
+ given(databaseStateVerifier.isLiquibaseDisabled()).willReturn(true);
+ });
+
+ When("After properties set is executed", () -> {
Review comment:
Wouldn't it make more sense to refer to a business event here instead of "afterpropertiesset"? I'm thinking about `When("The database migration process is executed")`
##########
File path: fineract-provider/src/test/java/org/apache/fineract/infrastructure/classpath/ClasspathDuplicatesScenario.java
##########
@@ -16,84 +16,64 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.fineract.infrastructure.classdupes;
+package org.apache.fineract.infrastructure.classpath;
+import io.cucumber.java8.En;
import io.github.classgraph.ClassGraph;
import io.github.classgraph.ResourceList;
import io.github.classgraph.ScanResult;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
+import org.opentest4j.AssertionFailedError;
-/**
- * Check classpath for duplicates.
- *
- * @author Michael Vorburger.ch
- */
-public class ClasspathHellDuplicatesChecker {
-
- public static final ClasspathHellDuplicatesChecker INSTANCE = new ClasspathHellDuplicatesChecker();
-
- private final Map<String, List<String>> duplicates;
-
- public ClasspathHellDuplicatesChecker() {
- duplicates = recheck();
- }
+public class ClasspathDuplicatesScenario implements En {
- public Map<String, List<String>> getDuplicates() {
- return duplicates;
- }
+ private Map<String, List<String>> duplicates = new HashMap<>();
- public String toString(Map<String, List<String>> map) {
- StringBuilder sb = new StringBuilder();
- for (Map.Entry<String, List<String>> entry : map.entrySet()) {
- sb.append(entry.getKey());
- sb.append('\n');
- for (String location : entry.getValue()) {
- sb.append(" ");
- sb.append(location);
- sb.append('\n');
- }
- }
- return sb.toString();
- }
+ public ClasspathDuplicatesScenario() {
+ Given("A classpath", () -> {
+ // nothing to do here
+ });
- private Map<String, List<String>> recheck() {
- Map<String, List<String>> dupes = new HashMap<>();
- // To debug this scanner, use ClassGraph().verbose()
- // We intentionally do not use .classFilesOnly(), or
- // .nonClassFilesOnly(), to check both
- try (ScanResult scanResult = new ClassGraph().scan()) {
- for (Map.Entry<String, ResourceList> dupe : scanResult.getAllResources().findDuplicatePaths()) {
- String resourceName = dupe.getKey();
- if (!isHarmlessDuplicate(resourceName)) {
- boolean addIt = true;
- List<String> jars = dupe.getValue().stream().map(resource -> resource.getURL().toExternalForm())
- .collect(Collectors.toList());
- for (String jar : jars) {
- if (skipJAR(jar)) {
- addIt = false;
- break;
+ When("The user scans the classpath", () -> {
+ // nothing to do here
Review comment:
Copy paste I guess.
##########
File path: fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/LiquibaseDisabledScenario.java
##########
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.fineract.infrastructure.core;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.BDDMockito.given;
+
+import com.zaxxer.hikari.HikariDataSource;
+import io.cucumber.java8.En;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.service.migration.ExtendedSpringLiquibaseFactory;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDataSourceFactory;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseStateVerifier;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseUpgradeService;
+import org.apache.fineract.infrastructure.security.service.TenantDetailsService;
+import org.springframework.beans.factory.annotation.Autowired;
+
+public class LiquibaseDisabledScenario implements En {
+
+ @Autowired
+ private TenantDetailsService tenantDetailsService;
+
+ @Autowired
+ private HikariDataSource tenantDataSource;
+
+ @Autowired
+ private FineractProperties fineractProperties;
+
+ @Autowired
+ private TenantDatabaseStateVerifier databaseStateVerifier;
+
+ @Autowired
+ private ExtendedSpringLiquibaseFactory liquibaseFactory;
+
+ @Autowired
+ private TenantDataSourceFactory tenantDataSourceFactory;
+
+ @Autowired
+ private TenantDatabaseUpgradeService underTest;
+
+ public LiquibaseDisabledScenario() {
+ Given("Liquibase is disabled", () -> {
+ given(databaseStateVerifier.isLiquibaseDisabled()).willReturn(true);
+ });
+
+ When("After properties set is executed", () -> {
+ underTest.afterPropertiesSet();
+ });
+
+ Then("The Liquibase configuration is unchanged", () -> {
Review comment:
`Then("The database migration has been skipped")` or something like that.
##########
File path: fineract-provider/src/test/java/org/apache/fineract/batch/builder/BatchBuilderScenario.java
##########
@@ -0,0 +1,62 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.fineract.batch.builder;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import io.cucumber.java8.En;
+import java.util.Collections;
+import java.util.List;
+import javax.ws.rs.core.UriInfo;
+import org.apache.fineract.batch.domain.BatchRequest;
+import org.apache.fineract.batch.domain.BatchResponse;
+import org.apache.fineract.batch.service.BatchApiService;
+
+public class BatchBuilderScenario implements En {
+
+ private BatchApiService batchApiService;
+
+ private List<BatchRequest> requests;
+
+ private UriInfo uriInfo;
+
+ private List<BatchResponse> responses;
+
+ public BatchBuilderScenario() {
+ Given("A batch request", () -> {
+ this.batchApiService = mock(BatchApiService.class);
+
+ this.uriInfo = mock(UriInfo.class);
+
+ this.requests = Collections.singletonList(new BatchRequest());
+ });
+
+ When("The user calls the batch service handle request method", () -> {
+ responses = this.batchApiService.handleBatchRequestsWithoutEnclosingTransaction(this.requests, uriInfo);
+ });
+
+ Then("The batch service handle request method should have been called", () -> {
Review comment:
Same as above.
##########
File path: fineract-provider/src/test/java/org/apache/fineract/accounting/common/AccountingFinanciaActivityScenario.java
##########
@@ -18,14 +18,25 @@
*/
package org.apache.fineract.accounting.common;
-import org.junit.jupiter.api.Test;
-import org.springframework.util.Assert;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
-public class AccountingConstantsTest {
+import io.cucumber.java8.En;
+import java.util.List;
+import org.apache.fineract.accounting.financialactivityaccount.data.FinancialActivityData;
- @Test
- public void testGetAllFinancialActivities() {
- Assert.notEmpty(AccountingConstants.FinancialActivity.getAllFinancialActivities(),
- "static initialization of collection of all enums is broken");
+public class AccountingFinanciaActivityScenario implements En {
+
+ private List<FinancialActivityData> data;
+
+ public AccountingFinanciaActivityScenario() {
+ Given("All financial activities", () -> {
+ this.data = AccountingConstants.FinancialActivity.getAllFinancialActivities();
+ });
+
+ Then("The they should not be empty", () -> {
+ assertNotNull(data);
+ assertFalse(data.isEmpty());
Review comment:
Again, it's just a styling question but I like to use the AssertJ fluent API for assertion, much more readable.
`assertThat(data).isNotEmpty()`
Thoughts?
##########
File path: fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/TenantMigrationFlywayFailNotLatestVersionScenario.java
##########
@@ -0,0 +1,103 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.fineract.infrastructure.core;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.BDDMockito.verify;
+import static org.mockito.BDDMockito.verifyNoInteractions;
+import static org.mockito.BDDMockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.mock;
+
+import com.zaxxer.hikari.HikariDataSource;
+import java.util.List;
+import javax.sql.DataSource;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.domain.FineractPlatformTenant;
+import org.apache.fineract.infrastructure.core.service.migration.ExtendedSpringLiquibase;
+import org.apache.fineract.infrastructure.core.service.migration.ExtendedSpringLiquibaseFactory;
+import org.apache.fineract.infrastructure.core.service.migration.SchemaUpgradeNeededException;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDataSourceFactory;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseStateVerifier;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseUpgradeService;
+import org.apache.fineract.infrastructure.security.service.TenantDetailsService;
+import org.springframework.beans.factory.annotation.Autowired;
+
+public class TenantMigrationFlywayFailNotLatestVersionScenario {
Review comment:
Let's merge this with the other Liquibase step defs.
##########
File path: fineract-provider/src/test/java/org/apache/fineract/accounting/common/AccountingFinanciaActivityScenario.java
##########
@@ -18,14 +18,25 @@
*/
package org.apache.fineract.accounting.common;
-import org.junit.jupiter.api.Test;
-import org.springframework.util.Assert;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
-public class AccountingConstantsTest {
+import io.cucumber.java8.En;
+import java.util.List;
+import org.apache.fineract.accounting.financialactivityaccount.data.FinancialActivityData;
- @Test
- public void testGetAllFinancialActivities() {
- Assert.notEmpty(AccountingConstants.FinancialActivity.getAllFinancialActivities(),
- "static initialization of collection of all enums is broken");
+public class AccountingFinanciaActivityScenario implements En {
Review comment:
Minor thing but do you think it would make sense to rename the class to `AccountingFinancialActivityStepDefinitions`?
Essentially the scenario is defined as the feature file and this class just contains the step definitions, hence the thought.
Of course this naming would be applicable for all other step definitions but I won't comment the same on all of them.
##########
File path: fineract-provider/src/test/resources/features/infrastructure/infrastructure.migration.feature
##########
@@ -0,0 +1,9 @@
+Feature: Migration Service
+
+ @migration @ignore
Review comment:
Just putting a mark here to avoid forgetting leaving the `@ignore` here.
##########
File path: fineract-provider/src/test/resources/features/infrastructure/infrastructure.core.feature
##########
@@ -0,0 +1,18 @@
+Feature: Core Infrastructure
+
+ # TODO: do we really have to test this?
+ @infrastructure @ignore
Review comment:
It's essentially verifying that if somebody disables Liquibase migrations, the database won't be touched. I think it's necesssary.
##########
File path: fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/TenantMigrationFlywayFailNotLatestVersionScenario.java
##########
@@ -0,0 +1,103 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.fineract.infrastructure.core;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.BDDMockito.verify;
+import static org.mockito.BDDMockito.verifyNoInteractions;
+import static org.mockito.BDDMockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.mock;
+
+import com.zaxxer.hikari.HikariDataSource;
+import java.util.List;
+import javax.sql.DataSource;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.domain.FineractPlatformTenant;
+import org.apache.fineract.infrastructure.core.service.migration.ExtendedSpringLiquibase;
+import org.apache.fineract.infrastructure.core.service.migration.ExtendedSpringLiquibaseFactory;
+import org.apache.fineract.infrastructure.core.service.migration.SchemaUpgradeNeededException;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDataSourceFactory;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseStateVerifier;
+import org.apache.fineract.infrastructure.core.service.migration.TenantDatabaseUpgradeService;
+import org.apache.fineract.infrastructure.security.service.TenantDetailsService;
+import org.springframework.beans.factory.annotation.Autowired;
+
+public class TenantMigrationFlywayFailNotLatestVersionScenario {
+
+ @Autowired
+ private TenantDetailsService tenantDetailsService;
+
+ @Autowired
+ private HikariDataSource tenantDataSource;
+
+ @Autowired
+ private FineractProperties fineractProperties;
+
+ @Autowired
+ private TenantDatabaseStateVerifier databaseStateVerifier;
+
+ @Autowired
+ private ExtendedSpringLiquibaseFactory liquibaseFactory;
+
+ @Autowired
+ private TenantDataSourceFactory tenantDataSourceFactory;
+
+ @Autowired
+ private TenantDatabaseUpgradeService tenantDatabaseUpgradeService;
+
+ public TenantMigrationFlywayFailNotLatestVersionScenario() throws Exception {
+ // TODO: @galovics need your help here
+
+ // given
+ ExtendedSpringLiquibase initialTenantStoreLiquibase = mock(ExtendedSpringLiquibase.class);
Review comment:
```
Given("Liquibase is enabled with a default tenant", () -> {
initialTenantStoreLiquibase = mock(ExtendedSpringLiquibase.class);
tenantStoreLiquibase = mock(ExtendedSpringLiquibase.class);
initialTenantLiquibase = mock(ExtendedSpringLiquibase.class);
tenantLiquibase = mock(ExtendedSpringLiquibase.class);
defaultTenant = mock(FineractPlatformTenant.class);
allTenants = List.of(defaultTenant);
defaultTenantDataSource = mock(DataSource.class);
given(databaseStateVerifier.isLiquibaseDisabled()).willReturn(false);
given(fineractProperties.getTenant()).willReturn(new FineractProperties.FineractTenantProperties());
given(liquibaseFactory.create(tenantDataSource, "tenant_store_db", "initial_switch")).willReturn(initialTenantStoreLiquibase);
given(liquibaseFactory.create(tenantDataSource, "tenant_store_db")).willReturn(tenantStoreLiquibase);
given(tenantDetailsService.findAllTenants()).willReturn(allTenants);
given(tenantDataSourceFactory.create(defaultTenant)).willReturn(defaultTenantDataSource);
given(liquibaseFactory.create(defaultTenantDataSource, "tenant_db", "initial_switch")).willReturn(initialTenantLiquibase);
given(liquibaseFactory.create(defaultTenantDataSource, "tenant_db")).willReturn(tenantLiquibase);
}
Given("Liquibase runs the very first time for the tenant store", () -> {
given(databaseStateVerifier.isFirstLiquibaseMigration(tenantDataSource)).willReturn(true);
}
Given("A previously Flyway migrated tenant store database", () -> {
given(databaseStateVerifier.isFlywayPresent(tenantDataSource)).willReturn(true);
}
Given("A previously Flyway migrated tenant store database on an earlier version than 1.6", () -> {
given(databaseStateVerifier.isTenantStoreOnLatestUpgradableVersion(tenantDataSource)).willReturn(false);
}
Given("Liquibase runs the very first time for the default tenant", () -> {
given(databaseStateVerifier.isFirstLiquibaseMigration(defaultTenantDataSource)).willReturn(true);
}
Given("A previously Flyway migrated default tenant database", () -> {
given(databaseStateVerifier.isFlywayPresent(defaultTenantDataSource)).willReturn(true);
}
Given("A previously Flyway migrated default tenant database on the 1.6 version", () -> {
given(databaseStateVerifier.isTenantStoreOnLatestUpgradableVersion(defaultTenantDataSource)).willReturn(true);
}
```
And we can change the `When("The database migration process is executed")` with the capability to handle exceptions so later in a Then we can verify it.
```
Then("The tenant store upgrade fails with a schema upgrade needed", () -> {
assertThat(result).isNotNull();
verify(liquibaseFactory).create(eq(tenantDataSource), any());
verifyNoMoreInteractions(liquibaseFactory);
verifyNoInteractions(initialTenantStoreLiquibase, tenantStoreLiquibase, initialTenantLiquibase, tenantLiquibase);
}
```
Sorry for the formatting, but you get the idea.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org