From 42dc87cba9c5e613c842f4a6d782d3434cfd6365 Mon Sep 17 00:00:00 2001 From: Denis Zhdanov Date: Thu, 26 May 2011 14:49:38 +0400 Subject: [PATCH] IDEA-70123 Test Assistant: Make it possible to work with test data for existing test classes that are not properly annotated Improved test data file guessing (e.g. our project has a lot of tests and test data files that conform to '*InComment*' pattern) --- .../TestDataGuessByExistingFilesUtil.java | 193 ++++++++++++++---- .../TestDataReferenceCollector.java | 3 + 2 files changed, 151 insertions(+), 45 deletions(-) diff --git a/plugins/IdeaTestAssistant/src/com/intellij/testAssistant/TestDataGuessByExistingFilesUtil.java b/plugins/IdeaTestAssistant/src/com/intellij/testAssistant/TestDataGuessByExistingFilesUtil.java index 722d8ed622c1..369f0cf13fae 100644 --- a/plugins/IdeaTestAssistant/src/com/intellij/testAssistant/TestDataGuessByExistingFilesUtil.java +++ b/plugins/IdeaTestAssistant/src/com/intellij/testAssistant/TestDataGuessByExistingFilesUtil.java @@ -4,6 +4,7 @@ import com.intellij.codeInsight.AnnotationUtil; import com.intellij.ide.util.gotoByName.GotoFileModel; import com.intellij.openapi.extensions.Extensions; import com.intellij.openapi.util.Pair; +import com.intellij.openapi.util.Trinity; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiClass; import com.intellij.psi.PsiElement; @@ -12,6 +13,7 @@ import com.intellij.psi.PsiMethod; import com.intellij.psi.codeStyle.NameUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.testIntegration.TestFramework; +import com.intellij.util.PathUtil; import com.intellij.util.containers.ConcurrentHashMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -25,23 +27,23 @@ import java.util.concurrent.TimeUnit; /** * There is a possible case that particular test class is not properly configured with test annotations but uses test data files. * This class contains utility methods for guessing test data files location and name patterns from existing one. - * + * * @author Denis Zhdanov * @since 5/24/11 2:28 PM */ public class TestDataGuessByExistingFilesUtil { private static final long CACHE_ENTRY_TTL_MS = TimeUnit.MILLISECONDS.convert(5, TimeUnit.MINUTES); - + private static final Map> CACHE = new ConcurrentHashMap>(); - + private TestDataGuessByExistingFilesUtil() { } /** * Tries to guess what test data files match to the given method if it's test method and there are existing test data * files for the target test class. - * + * * @param method test method candidate * @return collection of paths to the test data files for the given test if it's possible to guess them; * null otherwise @@ -57,7 +59,7 @@ public class TestDataGuessByExistingFilesUtil { } return collectTestDataByExistingFiles(psiFile, getTestName(method.getName())); } - + @Nullable private static T getParent(@NotNull PsiElement element, Class clazz) { for (PsiElement e = element; e != null; e = e.getParent()) { @@ -67,7 +69,7 @@ public class TestDataGuessByExistingFilesUtil { } return null; } - + @Nullable static List collectTestDataByExistingFiles(@NotNull PsiFile psiFile, @NotNull String testName) { GotoFileModel model = new GotoFileModel(psiFile.getProject()); @@ -78,7 +80,7 @@ public class TestDataGuessByExistingFilesUtil { return descriptor.generate(testName); } - + @Nullable private static String getTestName(@NotNull PsiMethod method) { final PsiClass psiClass = getParent(method, PsiClass.class); @@ -98,7 +100,7 @@ public class TestDataGuessByExistingFilesUtil { if (framework == null || isUtilityMethod(method, psiClass, framework)) { return null; } - + return getTestName(method.getName()); } @@ -106,19 +108,19 @@ public class TestDataGuessByExistingFilesUtil { if (method == framework.findSetUpMethod(psiClass) || method == framework.findTearDownMethod(psiClass)) { return true; } - + // JUnit3 if (framework.getClass().getName().contains("JUnit3")) { return !method.getName().startsWith("test"); } - + // JUnit4 else if (framework.getClass().getName().contains("JUnit4")) { return !AnnotationUtil.isAnnotated(method, "org.junit.Test", false); } return false; } - + @NotNull public static String getTestName(@NotNull String methodName) { return methodName.startsWith("test") ? methodName.substring("test".length()) : methodName; @@ -134,7 +136,7 @@ public class TestDataGuessByExistingFilesUtil { final Pair cached = CACHE.get(psiClass.getQualifiedName()); if (cached != null && cached.second + CACHE_ENTRY_TTL_MS > System.currentTimeMillis()) { return cached.first.isComplete() ? cached.first : null; - } + } TestFramework[] frameworks = Extensions.getExtensions(TestFramework.EXTENSION_NAME); TestFramework framework = null; @@ -151,22 +153,28 @@ public class TestDataGuessByExistingFilesUtil { final PsiElement setUpMethod = framework.findSetUpMethod(psiClass); final PsiElement tearDownMethod = framework.findTearDownMethod(psiClass); TestDataDescriptor descriptor = new TestDataDescriptor(); + List testNames = new ArrayList(); for (PsiMethod method : psiClass.getMethods()) { final String name = getTestName(method.getName()); - if (method == setUpMethod || method == tearDownMethod || name.equals(psiClass.getName())) { + if (method == setUpMethod || method == tearDownMethod || name.equals(psiClass.getName()) + || isUtilityMethod(method, psiClass, framework)) + { continue; } - final Collection matchedFiles = getMatchedFiles(gotoModel, name); - if (!descriptor.isComplete()) { - descriptor.populate(name, matchedFiles); - } - if (descriptor.isComplete()) { - CACHE.put(psiClass.getQualifiedName(), new Pair(descriptor, System.currentTimeMillis())); - return descriptor; - } + testNames.add(name); } - CACHE.put(psiClass.getQualifiedName(), new Pair(new TestDataDescriptor(), System.currentTimeMillis())); - return null; + final Pair> matchedFiles = getMatchedFiles(gotoModel, testNames, psiClass); + if (matchedFiles == null) { + CACHE.put(psiClass.getQualifiedName(), new Pair(descriptor, System.currentTimeMillis())); + return descriptor; + } + descriptor.populate(matchedFiles.first, matchedFiles.second); + if (!descriptor.isComplete()) { + CACHE.put(psiClass.getQualifiedName(), new Pair(descriptor, System.currentTimeMillis())); + return null; + } + CACHE.put(psiClass.getQualifiedName(), new Pair(descriptor, System.currentTimeMillis())); + return descriptor; } //@NotNull @@ -196,28 +204,130 @@ public class TestDataGuessByExistingFilesUtil { // }, project); // return result; //} - - @NotNull - private static Collection getMatchedFiles(@NotNull GotoFileModel gotoModel, @NotNull String testName) { - String pattern = String.format("*%s*", testName); - final NameUtil.Matcher matcher = NameUtil.buildMatcher(pattern, 0, true, true, pattern.toLowerCase().equals(pattern)); - List result = new ArrayList(); + + @Nullable + private static Pair> getMatchedFiles(@NotNull GotoFileModel gotoModel, + @NotNull Collection testNames, + @NotNull PsiClass psiClass) + { + List> input = new ArrayList>(); + for (String testName : testNames) { + String pattern = String.format("*%s*", testName); + input.add(new Trinity( + NameUtil.buildMatcher(pattern, 0, true, true, pattern.toLowerCase().equals(pattern)), testName, pattern + )); + } + String dir = null; + String testName = null; + List files = new ArrayList(); for (String name : gotoModel.getNames(false)) { - if (matcher.matches(name)) { - final Object[] elements = gotoModel.getElementsByName(name, false, pattern); - if (elements != null) { - for (Object element : elements) { - if (element instanceof PsiFile) { - result.add(((PsiFile)element).getVirtualFile()); - } + boolean currentNameProcessed = false; + for (Trinity trinity : input) { + if (!trinity.first.matches(name)) { + continue; + } + + final Object[] elements = gotoModel.getElementsByName(name, false, trinity.third); + if (elements == null) { + continue; + } + for (Object element : elements) { + if (!(element instanceof PsiFile)) { + continue; } + final VirtualFile file = ((PsiFile)element).getVirtualFile(); + if (file == null) { + continue; + } + + final String filePath = PathUtil.getFileName(file.getPath()).toLowerCase(); + int i = filePath.indexOf(trinity.second.toLowerCase()); + // Skip files that doesn't contain target test name and files that contain digit after target test name fragment. + // Example: there are tests with names 'testEnter()' and 'testEnter2()' and we don't want test data file 'testEnter2' + // to be matched to the test 'testEnter()'. + if (i < 0 || (i + trinity.second.length() < filePath.length()) + && Character.isDigit(filePath.charAt(i + trinity.second.length()))) + { + continue; + } + + currentNameProcessed = true; + final String parentPath = PathUtil.getParentPath(file.getPath()); + if (dir == null || dir.equals(parentPath)) { + dir = parentPath; + if (testName == null || !testName.equals(trinity.second)) { + files.clear(); + } + testName = trinity.second; + files.add(file); + continue; + } + if (moreRelevantPath(file, files, psiClass, trinity.second)) { + testName = trinity.second; + dir = parentPath; + files.clear(); + files.add(file); + } + } + if (currentNameProcessed) { + break; } } } - return result; + return (testName == null || files.isEmpty()) ? null : new Pair>(testName, files); + } + + private static boolean moreRelevantPath(@NotNull VirtualFile candidate, @NotNull List current, @NotNull PsiClass psiClass, + @NotNull String testName) + { + final String className = psiClass.getQualifiedName(); + if (className == null) { + return false; + } + + final String candidatePath = candidate.getPath(); + final String candidateDir = PathUtil.getParentPath(candidatePath); + final String currentDir = PathUtil.getParentPath(current.get(0).getPath()); + + // By package. + int i = className.lastIndexOf("."); + if (i >= 0) { + String packageAsPath = className.substring(0, i).replace('.', '/').toLowerCase(); + if (candidateDir.toLowerCase().contains(packageAsPath) && !currentDir.toLowerCase().contains(packageAsPath)) { + return true; + } + } + + // By class name. + String pattern = className.toLowerCase(); + if (pattern.endsWith("test")) { + pattern = pattern.substring(0, pattern.length() - "Test".length()); + } + i = pattern.lastIndexOf('.'); + if (i >= 0) { + pattern = pattern.substring(i + 1); + } + if (candidateDir.toLowerCase().contains(pattern) && !currentDir.toLowerCase().contains(pattern)) { + return true; + } + + // By test name. + if (PathUtil.getFileName(candidatePath).toLowerCase().startsWith(testName.toLowerCase())) { + boolean moreRelevant = true; + for (VirtualFile file : current) { + if (PathUtil.getFileName(file.getPath()).toLowerCase().startsWith(testName.toLowerCase())) { + moreRelevant = false; + break; + } + } + if (moreRelevant) { + return true; + } + } + + return false; } - private static class TestLocationDescriptor { public String dir; @@ -245,13 +355,6 @@ public class TestDataGuessByExistingFilesUtil { startWithLowerCase = testNameStartsWithLowerCase; } - // Skip files that doesn't contain target test name and files that contain digit after target test name fragment. - // Example: there are tests with names 'testEnter()' and 'testEnter2()' and we don't want test data file 'testEnter2' - // to be matched to the test 'testEnter()'. - if (i < 0 || (i + testName.length() < fileName.length()) && Character.isDigit(fileName.charAt(i + testName.length()))) { - return; - } - filePrefix = fileName.substring(0, i); fileSuffix = fileName.substring(i + testName.length()); ext = matched.getExtension(); diff --git a/plugins/IdeaTestAssistant/src/com/intellij/testAssistant/TestDataReferenceCollector.java b/plugins/IdeaTestAssistant/src/com/intellij/testAssistant/TestDataReferenceCollector.java index 30b643818296..b08af991e85a 100644 --- a/plugins/IdeaTestAssistant/src/com/intellij/testAssistant/TestDataReferenceCollector.java +++ b/plugins/IdeaTestAssistant/src/com/intellij/testAssistant/TestDataReferenceCollector.java @@ -55,6 +55,9 @@ public class TestDataReferenceCollector { private List collectTestDataReferences(final PsiMethod method, final Map> argumentMap) { final List result = new ArrayList(); + if (myTestDataPath == null) { + return result; + } method.accept(new JavaRecursiveElementVisitor() { @Override public void visitMethodCallExpression(PsiMethodCallExpression expression) {