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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
