From 7584ef4e13d82b7806c6f02317c0966be4084f52 Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Wed, 23 Apr 2014 17:49:52 +1000 Subject: [PATCH] Move and simplify tests from CopyTransServiceImplTest to TranslationFinderTest Remove CopyTransServiceImplTest combinations which are tested by TranslationFinderTest --- .../impl/CopyTransServiceImplTest.java | 69 ++----- .../zanata/service/impl/ExecutionHelper.java | 68 +++++++ .../service/impl/TranslationFinderTest.java | 170 ++++++++++++++++++ .../TranslationMemoryServiceImplTest.java | 9 +- 4 files changed, 257 insertions(+), 59 deletions(-) create mode 100644 zanata-war/src/test/java/org/zanata/service/impl/ExecutionHelper.java diff --git a/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java b/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java index ca8c4f1bc0..c7ded5f5c6 100644 --- a/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java +++ b/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java @@ -65,10 +65,12 @@ import static org.zanata.model.HCopyTransOptions.ConditionRuleAction.DOWNGRADE_TO_FUZZY; import static org.zanata.model.HCopyTransOptions.ConditionRuleAction.IGNORE; import static org.zanata.model.HCopyTransOptions.ConditionRuleAction.REJECT; +import static org.zanata.service.impl.ExecutionHelper.cartesianProduct; /** * @author Carlos Munoz camunoz@redhat.com + * @author Sean Flanigan sflaniga@redhat.com */ @Test(groups = { "business-tests" }) public class CopyTransServiceImplTest extends ZanataDbunitJpaTest { @@ -117,12 +119,9 @@ public void individualTest() { .expectTransState(NeedReview)); } - // TODO move most of the cases into TranslationMemoryServiceImplTest - // FIXME cut down the number of executions, and remove the 'indices' param - // NB: Reduced because it otherwise takes about 25 minutes - @DataProvider(name = "CopyTrans", indices = {0,1,2,3,4,5,6,7,8}) + @DataProvider(name = "CopyTrans") protected Object[][] createCopyTransTestParams() { - Set expandedExecutions = generateAllExecutions(); + Set expandedExecutions = generateExecutions(); Object[][] val = new Object[expandedExecutions.size()][1]; int i = 0; @@ -170,7 +169,7 @@ public void testCopyTrans(CopyTransExecution execution) { // Create the document HDocument doc = new HDocument(); doc.setContentType(ContentType.TextPlain); - doc.setLocale(localeDAO.findByLocaleId(new LocaleId("en-US"))); + doc.setLocale(localeDAO.findByLocaleId(LocaleId.EN_US)); doc.setProjectIteration(projectIteration); if (execution.documentMatches) { doc.setFullPath("/same/document"); @@ -213,10 +212,8 @@ public void testCopyTrans(CopyTransExecution execution) { .setParameter("docId", doc.getDocId()) .setParameter("resId", textFlow.getResId()) .getSingleResult(); - HTextFlowTarget target = targetTextFlow.getTargets().get(3L); // Id: 3 - // for - // Locale - // de + // Id: 3L for Locale de + HTextFlowTarget target = targetTextFlow.getTargets().get(3L); if (execution.isExpectUntranslated()) { if (target != null && target.getState() != ContentState.New) { @@ -305,8 +302,7 @@ public void reuseTranslationsFromObsoleteDocuments() throws Exception { } } - // Run the copy trans scenario (very liberal, but nothing should be - // translated) + // Run the copy trans scenario CopyTransExecution execution = new CopyTransExecution(IGNORE, IGNORE, IGNORE, true, true, true, true, Approved).expectTransState(Approved); @@ -346,15 +342,16 @@ private static ContentState getExpectedContentState(boolean match, } } - private Set generateAllExecutions() { + private Set generateExecutions() { Set allExecutions = new HashSet(); + // NB combinations which affect the query parameters + // (context match/mismatch, etc) are tested in TranslationFinderTest Set paramsSet = - cartesianProduct(Arrays.asList(ConditionRuleAction.values()), - Arrays.asList(ConditionRuleAction.values()), - Arrays.asList(ConditionRuleAction.values()), - Arrays.asList(true, false), Arrays.asList(true, false), - Arrays.asList(true, false), Arrays.asList(true, false), + cartesianProduct(Arrays.asList(REJECT), + Arrays.asList(REJECT), Arrays.asList(REJECT), + Arrays.asList(true), Arrays.asList(true), + Arrays.asList(true), Arrays.asList(true, false), Arrays.asList(Translated, Approved)); for (Object[] params : paramsSet) { @@ -379,41 +376,6 @@ private Set generateAllExecutions() { return allExecutions; } - /** - * Utility method to generate a cartesian product of all possible scenarios - */ - private Set cartesianProduct(Iterable... colls) { - // Base case - if (colls.length == 1) { - Iterable lastSet = colls[0]; - Set product = new HashSet(); - - for (Object elem : lastSet) { - product.add(new Object[] { elem }); - } - return product; - } - // Recursive case - else { - Iterable lastSet = colls[colls.length - 1]; - Set subProduct = - cartesianProduct(Arrays.copyOfRange(colls, 0, - colls.length - 1)); - Set fullProduct = new HashSet(); - - for (Object[] subProdElem : subProduct) { - for (Object elem : lastSet) { - Object[] newSubProd = - Arrays.copyOf(subProdElem, subProdElem.length + 1); - newSubProd[newSubProd.length - 1] = elem; - fullProduct.add(newSubProd); - } - } - - return fullProduct; - } - } - @Getter @Setter @EqualsAndHashCode @@ -469,6 +431,7 @@ public CopyTransExecution withContents(String... contents) { return this; } + // TODO do we still want this? public Collection expand() { Set expanded = new HashSet(); diff --git a/zanata-war/src/test/java/org/zanata/service/impl/ExecutionHelper.java b/zanata-war/src/test/java/org/zanata/service/impl/ExecutionHelper.java new file mode 100644 index 0000000000..609d592e06 --- /dev/null +++ b/zanata-war/src/test/java/org/zanata/service/impl/ExecutionHelper.java @@ -0,0 +1,68 @@ +/* + * Copyright 2014, Red Hat, Inc. and individual contributors + * as indicated by the @author tags. See the copyright.txt file in the + * distribution for a full listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.zanata.service.impl; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +/** + * @author Carlos Munoz camunoz@redhat.com + */ +class ExecutionHelper { + + /** + * Utility method to generate a cartesian product of all possible scenarios + */ + static Set cartesianProduct(Iterable... colls) { + // Base case + if (colls.length == 1) { + Iterable lastSet = colls[0]; + Set product = new HashSet(); + + for (Object elem : lastSet) { + product.add(new Object[] { elem }); + } + return product; + } + // Recursive case + else { + Iterable lastSet = colls[colls.length - 1]; + Set subProduct = + cartesianProduct(Arrays.copyOfRange(colls, 0, + colls.length - 1)); + Set fullProduct = new HashSet(); + + for (Object[] subProdElem : subProduct) { + for (Object elem : lastSet) { + Object[] newSubProd = + Arrays.copyOf(subProdElem, subProdElem.length + 1); + newSubProd[newSubProd.length - 1] = elem; + fullProduct.add(newSubProd); + } + } + + return fullProduct; + } + } + +} diff --git a/zanata-war/src/test/java/org/zanata/service/impl/TranslationFinderTest.java b/zanata-war/src/test/java/org/zanata/service/impl/TranslationFinderTest.java index 85265d2108..ff5e56dd72 100644 --- a/zanata-war/src/test/java/org/zanata/service/impl/TranslationFinderTest.java +++ b/zanata-war/src/test/java/org/zanata/service/impl/TranslationFinderTest.java @@ -1,9 +1,14 @@ package org.zanata.service.impl; import static org.assertj.core.api.Assertions.assertThat; +import static org.zanata.service.impl.ExecutionHelper.cartesianProduct; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import lombok.Data; import org.assertj.core.api.Condition; import org.dbunit.operation.DatabaseOperation; import org.hibernate.search.impl.FullTextSessionImpl; @@ -11,12 +16,16 @@ import org.mockito.MockitoAnnotations; import org.testng.Assert; import org.testng.annotations.BeforeClass; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import org.zanata.ImmutableDbunitJpaTest; +import org.zanata.common.ContentType; import org.zanata.common.LocaleId; +import org.zanata.dao.LocaleDAO; import org.zanata.dao.ProjectIterationDAO; import org.zanata.dao.TextFlowTargetDAO; import org.zanata.model.HDocument; +import org.zanata.model.HLocale; import org.zanata.model.HProjectIteration; import org.zanata.model.HTextFlow; import org.zanata.model.HTextFlowTarget; @@ -28,9 +37,13 @@ /** * @author Alex Eng aeng@redhat.com + * @author Carlos Munoz camunoz@redhat.com + * @author Sean Flanigan sflaniga@redhat.com */ public class TranslationFinderTest extends ImmutableDbunitJpaTest { private SeamAutowire seam = SeamAutowire.instance(); + private HLocale sourceLocale; @Override protected void prepareDBUnitOperations() { @@ -56,6 +69,9 @@ public void beforeClass() throws Exception { .useImpl(AsyncTaskManagerServiceImpl.class) .ignoreNonResolvable(); seam.autowire(SearchIndexManager.class).reindex(true, true, false); + LocaleDAO localeDAO = seam.autowire(LocaleDAO.class); + + sourceLocale = localeDAO.findByLocaleId(LocaleId.EN_US); } @Test @@ -110,4 +126,158 @@ public boolean matches(List strings) { }); } + /** + * Use this test to individually test copy trans scenarios. + */ + @Test(enabled = false) + public void individualTest() { + this.testCopyTransQuery(new Execution(false, true, true, true, true, true)); + } + + @Test(dataProvider = "CopyTrans") + public void testCopyTransQuery(Execution execution) { + TranslationFinder service = seam.autowire(TranslationMemoryServiceImpl.class); +// TranslationFinder service = seam.autowire(TextFlowTargetDAO.class); + + // Prepare Execution + ProjectIterationDAO iterationDAO = + seam.autowire(ProjectIterationDAO.class); + + // Get the project iteration + HProjectIteration queryProjIter = + iterationDAO.getBySlug(execution.getProject(), execution.getVersion()); + + assert queryProjIter != null; + + // Create the document + HDocument queryDoc = new HDocument(); + queryDoc.setContentType(ContentType.TextPlain); + queryDoc.setLocale(sourceLocale); + queryDoc.setProjectIteration(queryProjIter); + queryDoc.setFullPath(execution.getDocument()); + + // Create the text Flow + HTextFlow queryTextFlow = new HTextFlow(); + // TODO test that the query textflow is excluded from results + // (when the query textflow has been persisted and indexed) + queryTextFlow.setId(-999L); + queryTextFlow.setContents(execution.getContent()); // Source content matches + queryTextFlow.setPlural(false); + queryTextFlow.setObsolete(false); + queryTextFlow.setDocument(queryDoc); + queryTextFlow.setResId(execution.getContext()); + + // For all the executions whose queries are expected to find a match, + // the target which is expected to match is HTextFlowTarget 100 + // (which belongs to HTextFlow 100, HDocument 100, + // HProjectIteration 100, HProject 100) + + Optional matchingTarget = + service.searchBestMatchTransMemory( + queryTextFlow, LocaleId.DE, LocaleId.EN_US, + execution.isCheckContext(), execution.isCheckDocument(), + execution.isCheckProject()); + + assertThat(matchingTarget.isPresent()).as("match present").isEqualTo(execution.expectMatch()); + + if (matchingTarget.isPresent()) { + HTextFlowTarget target = matchingTarget.get(); + HTextFlow tf = target.getTextFlow(); + assertThat(target.getLocaleId()).isEqualTo(LocaleId.DE); + assertThat(tf.getContents()).containsExactly(execution.getContent()); + + if (execution.isCheckContext()) { + assertThat(tf.getResId()).isEqualTo(execution.getContext()); + } + + if (execution.isCheckDocument()) { + assertThat(tf.getDocument().getDocId()).isEqualTo(execution.getDocument()); + } + + if (execution.isCheckProject()) { + assertThat(tf.getDocument().getProjectIteration().getProject().getSlug()).isEqualTo( + execution.getProject()); + } + + // TODO check that we have the newest appropriate match + } + + + } + + @DataProvider(name = "CopyTrans") + protected Object[][] createCombinations() { + Set expandedExecutions = generateAllExecutions(); + + Object[][] val = new Object[expandedExecutions.size()][1]; + int i = 0; + for (Execution exe : expandedExecutions) { + val[i++][0] = exe; + } + + return val; + } + + private Set generateAllExecutions() { + Set allExecutions = + new HashSet(); + List booleans = Arrays.asList(true, false); + Set paramsSet = + cartesianProduct(booleans, booleans, booleans, booleans, + booleans, booleans, booleans); + + for (Object[] params : paramsSet) { + Execution exec = + new Execution( + (Boolean) params[0], (Boolean) params[1], + (Boolean) params[2], (Boolean) params[3], + (Boolean) params[4], (Boolean) params[5]); + allExecutions.add(exec); + } + return allExecutions; + } + + @Data + private static class Execution { + // including this field causes Lombok @ToString to include getMatch() + @SuppressWarnings("unused") + private String match; + + final boolean checkContext; + final boolean checkProject; + final boolean checkDocument; + final boolean matchingContext; + final boolean matchingProject; + final boolean matchingDocument; + + String getMatch() { + return String.valueOf(expectMatch()).toUpperCase(); + } + + boolean expectMatch() { + return (matchingContext || !checkContext) && + (matchingProject || !checkProject) && + (matchingDocument || !checkDocument); + } + + String getContent() { + return "Source Content"; + } + + String getContext() { + return matchingContext ? "same-context" : "different-context"; + } + + String getProject() { + return matchingProject ? "same-project" : "different-project"; + } + + String getVersion() { + return matchingProject ? "same-version" : "different-version"; + } + + String getDocument() { + return matchingDocument ? "/same/document0" : "/different/document"; + } + } } diff --git a/zanata-war/src/test/java/org/zanata/service/impl/TranslationMemoryServiceImplTest.java b/zanata-war/src/test/java/org/zanata/service/impl/TranslationMemoryServiceImplTest.java index d89dd930bd..d4da75aac8 100644 --- a/zanata-war/src/test/java/org/zanata/service/impl/TranslationMemoryServiceImplTest.java +++ b/zanata-war/src/test/java/org/zanata/service/impl/TranslationMemoryServiceImplTest.java @@ -3,12 +3,12 @@ import java.util.Date; import java.util.List; +import lombok.ToString; import org.assertj.core.api.Condition; import org.dbunit.operation.DatabaseOperation; import org.hibernate.search.impl.FullTextSessionImpl; import org.hibernate.search.jpa.Search; import org.mockito.MockitoAnnotations; -import org.testng.Assert; import org.testng.annotations.BeforeClass; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -38,10 +38,7 @@ import lombok.AllArgsConstructor; import lombok.Getter; -import static com.google.common.collect.Lists.newArrayList; import static org.assertj.core.api.Assertions.assertThat; -import static org.zanata.common.ContentState.Approved; -import static org.zanata.common.ContentState.Translated; /** * @author Alex Eng aeng@redhat.com @@ -79,9 +76,7 @@ public void beforeClass() throws Exception { .useImpl(AsyncTaskManagerServiceImpl.class) .ignoreNonResolvable() .autowire(TranslationMemoryServiceImpl.class); - seam.autowire(SearchIndexManager.class).reindex(true, true, false); - LocaleDAO localeDAO = seam.autowire(LocaleDAO.class); sourceLocale = localeDAO.findByLocaleId(LocaleId.EN_US); @@ -253,6 +248,7 @@ HasSearchType.SearchType.FUZZY, getCondition(checks[0], @Getter @AllArgsConstructor + @ToString private class TransMemoryExecution { private TransMemoryQuery query; @@ -305,4 +301,5 @@ private static void setProjectAndIterationSlug(HTextFlow hTextFlow, projectIteration.setProject(project); hTextFlow.getDocument().setProjectIteration(projectIteration); } + }