Skip to content

Commit

Permalink
dedupe the list of jar files from test param files (#388)
Browse files Browse the repository at this point in the history
  • Loading branch information
@plaird
plaird authored Jan 4, 2022
1 parent 1aea3c2 commit bb0b0bc
Show file tree
Hide file tree
Showing 4 changed files with 247 additions and 166 deletions.
64 changes: 48 additions & 16 deletions ...java-sdk/src/main/java/com/salesforce/bazel/sdk/lang/jvm/BazelJvmTestClasspathHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.util.Map;
import java.util.Scanner;
import java.util.Set;
import java.util.TreeSet;

import com.salesforce.bazel.sdk.command.BazelWorkspaceCommandRunner;
import com.salesforce.bazel.sdk.logging.LogHelper;
Expand All @@ -63,7 +64,7 @@ public class BazelJvmTestClasspathHelper {
static final String BAZEL_SRC_DEPLOY_PARAMS_SUFFIX = "_deploy-src.jar-0.params";

// Cache for test classpath computations. In some envs, this can result in huge performance benefits.
private static Map<Long, ParamFileResult> cachedResults = new HashMap<>();
private Map<Long, ParamFileResult> cachedResults = new HashMap<>();

// The TTL and LastFlush are public so that the tool can decide how much time is appropriate for the cache.
// You can lower/raise the TTL, or force a flush by setting cacheLastFlushMS to 0
Expand All @@ -76,7 +77,7 @@ public class BazelJvmTestClasspathHelper {
/**
* The jar suffix to be used to find the params file.
*/
public static String getParamsJarSuffix(boolean isSource) {
public String getParamsJarSuffix(boolean isSource) {
String suffix = BAZEL_DEPLOY_PARAMS_SUFFIX;
if (isSource) {
// TODO what is the use case for finding a test classpath for a src jar?
Expand All @@ -91,7 +92,7 @@ public static String getParamsJarSuffix(boolean isSource) {
* If a testClassName is passed, it will often speed up the operation as the param file for that test class can
* often be found on the file system.
*/
public static ParamFileResult findParamFilesForTests(BazelWorkspace bazelWorkspace, BazelProject bazelProject,
public ParamFileResult findParamFilesForTests(BazelWorkspace bazelWorkspace, BazelProject bazelProject,
boolean isSource, String testClassName, BazelProjectTargets targets) {
ParamFileResult result = null;

Expand All @@ -118,7 +119,7 @@ public static ParamFileResult findParamFilesForTests(BazelWorkspace bazelWorkspa
return result;
}

private static Long generateCacheKey(boolean isSource, String testClassName, BazelProjectTargets targets) {
private Long generateCacheKey(boolean isSource, String testClassName, BazelProjectTargets targets) {
long key = isSource ? 7 : 13;
if (testClassName != null) {
key = testClassName.hashCode() * key;
Expand All @@ -142,21 +143,21 @@ public static class ParamFileResult {
* <p>
* Internally, this method uses Bazel query, which is somewhat expensive.
*/
public static ParamFileResult findParamFilesForTestTargets(BazelWorkspace bazelWorkspace, BazelProject bazelProject,
public ParamFileResult findParamFilesForTestTargets(BazelWorkspace bazelWorkspace, BazelProject bazelProject,
boolean isSource, BazelProjectTargets targets) {
ParamFileResult result = new ParamFileResult();

File bazelBinDir = bazelWorkspace.getBazelBinDirectory();
String suffix = BazelJvmTestClasspathHelper.getParamsJarSuffix(isSource);
String suffix = getParamsJarSuffix(isSource);

for (String target : targets.getConfiguredTargets()) {
String query = "tests(" + target + ")";
List<String> labels = bazelWorkspace.getTargetsForBazelQuery(query);

for (String label : labels) {
String testRuleName = label.substring(label.lastIndexOf(":") + 1);
String targetPath = target.split(":")[0];
String paramFilename = testRuleName + suffix;
for (String label : labels) { // //projects/apple:src/test/java/com/foo/apple/AppleTest
String testRuleName = label.substring(label.lastIndexOf(":") + 1); // src/test/java/com/foo/apple/AppleTest
String targetPath = target.split(":")[0]; // //projects/apple
String paramFilename = testRuleName + suffix; // src/test/java/com/foo/apple/AppleTest_deploy.jar-0.params
File pFile = new File(new File(bazelBinDir, targetPath), paramFilename);
if (pFile.exists()) {
result.paramFiles.add(pFile);
Expand All @@ -172,14 +173,14 @@ public static ParamFileResult findParamFilesForTestTargets(BazelWorkspace bazelW
* Looks up the param files associated with the passed testclass. If this command needs to resort to a bazel query
* to find it, the scope of the bazel query commands will be the passed targets.
*/
public static ParamFileResult findParamFilesForTestClassname(BazelWorkspace bazelWorkspace,
public ParamFileResult findParamFilesForTestClassname(BazelWorkspace bazelWorkspace,
BazelProject bazelProject, boolean isSource, BazelProjectTargets targets, String testClassName) {
ParamFileResult result = new ParamFileResult();

String suffix = BazelJvmTestClasspathHelper.getParamsJarSuffix(isSource);
String suffix = getParamsJarSuffix(isSource);

for (String target : targets.getConfiguredTargets()) {
Set<File> testParamFiles = BazelJvmTestClasspathHelper.findParamsFileForTestClassnameAndTarget(
Set<File> testParamFiles = findParamsFileForTestClassnameAndTarget(
bazelWorkspace, bazelProject, target, testClassName, suffix);
result.paramFiles.addAll(testParamFiles);
}
Expand All @@ -190,7 +191,7 @@ public static ParamFileResult findParamFilesForTestClassname(BazelWorkspace baze
* Bazel maintains a params file for each java_test rule. It contains classpath info for the test. This method finds
* the params file.
*/
public static Set<File> findParamsFileForTestClassnameAndTarget(BazelWorkspace bazelWorkspace,
public Set<File> findParamsFileForTestClassnameAndTarget(BazelWorkspace bazelWorkspace,
BazelProject bazelProject, String target, String className, String suffix) {
// TODO in what case will there be multiple test param files?
Set<File> paramFiles = new HashSet<>();
Expand Down Expand Up @@ -276,6 +277,37 @@ public static Set<File> findParamsFileForTestClassnameAndTarget(BazelWorkspace b

return paramFiles;
}

/**
* Given the set of param files in the passed testParamFilesResult, parse each param file and extract a list
* of jar files from the sources and output sections of each file. Then assemble a de-duplicated list of these
* jar files. The path for each jar file comes from the param file and is known to be relative to the Bazel
* exec root of the workspace.
*/
public Set<String> aggregateJarFilesFromParamFiles(BazelJvmTestClasspathHelper.ParamFileResult testParamFilesResult) {
Set<String> allPaths = new TreeSet<>();
for (File paramsFile : testParamFilesResult.paramFiles) {
List<String> jarPaths = null;
try {
jarPaths = getClasspathJarsFromParamsFile(paramsFile);
} catch (IOException ioe) {
LOG.warn("Failed to parse test classpath file {}", paramsFile.getAbsolutePath());
}
if (jarPaths == null) {
// error has already been logged, just try to soldier on
continue;
}

for (String jarPath : jarPaths) {
// it is important to use a Set for allPaths, as the jarPaths will contain many dupes
// across ParamFiles and we only want each one listed once
allPaths.add(jarPath);
}
}
return allPaths;
}



// PARAMS FILE PARSING

Expand All @@ -293,7 +325,7 @@ EXAMPLE param file contents (redacted, only shows what we are looking for)
/**
* Parse the classpath jars from the given params file.
*/
public static List<String> getClasspathJarsFromParamsFile(File paramsFile) throws IOException {
public List<String> getClasspathJarsFromParamsFile(File paramsFile) throws IOException {
if (!paramsFile.exists()) {
return null;
}
Expand All @@ -306,7 +338,7 @@ public static List<String> getClasspathJarsFromParamsFile(File paramsFile) throw
* Parses the param file, looking for classpath entries. These entries point to the materialized files for the
* classpath for the test.
*/
public static List<String> getClasspathJarsFromParamsFile(Scanner scanner) {
public List<String> getClasspathJarsFromParamsFile(Scanner scanner) {
List<String> result = new ArrayList<>();
boolean addToResult = false;
while (scanner.hasNextLine()) {
Expand Down
127 changes: 65 additions & 62 deletions ...se.core/src/main/java/com/salesforce/bazel/eclipse/launch/BazelTestClasspathProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,16 @@
package com.salesforce.bazel.eclipse.launch;

import java.io.File;
import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Path;
import org.eclipse.core.runtime.Status;
import org.eclipse.debug.core.ILaunchConfiguration;
import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
import org.eclipse.jdt.core.IJavaProject;
Expand All @@ -57,8 +55,6 @@
import org.eclipse.jdt.launching.StandardClasspathProvider;
import org.eclipse.jface.dialogs.MessageDialog;
import org.eclipse.swt.widgets.Display;
import org.osgi.framework.Bundle;
import org.osgi.framework.FrameworkUtil;

import com.salesforce.bazel.eclipse.component.ComponentContext;
import com.salesforce.bazel.eclipse.component.EclipseBazelWorkspaceContext;
Expand All @@ -81,15 +77,16 @@ public class BazelTestClasspathProvider extends StandardClasspathProvider {
"com.salesforce.bazel.eclipse.launchconfig.sourcepathProvider";
public static final String BAZEL_CLASSPATH_PROVIDER = "com.salesforce.bazel.eclipse.launchconfig.classpathProvider";

private static final Bundle BUNDLE = FrameworkUtil.getBundle(BazelTestClasspathProvider.class);

// suppresses a common error dialog
private static int javaTestDialogSkipCount = 0;

// computeUnresolvedClassPathEntries() is called multiple times while trying to run a single test,
// we need this variable to keep track of when to open the error dialog
public static AtomicBoolean canOpenErrorDialog = new AtomicBoolean(true);

// collaborator for retrieving/analyzing Bazel test param files
BazelJvmTestClasspathHelper bazelJvmTestClasspathHelper = new BazelJvmTestClasspathHelper();

/**
* Compute classpath entries for test
*/
Expand Down Expand Up @@ -133,85 +130,56 @@ public IRuntimeClasspathEntry[] resolveClasspath(IRuntimeClasspathEntry[] entrie
*/
IRuntimeClasspathEntry[] computeUnresolvedClasspath(ILaunchConfiguration configuration, boolean isSource)
throws CoreException {
List<IRuntimeClasspathEntry> result = new ArrayList<>();
IJavaProject project = JavaRuntime.getJavaProject(configuration);
String projectName = project.getProject().getName();
BazelWorkspace bazelWorkspace = EclipseBazelWorkspaceContext.getInstance().getBazelWorkspace();
BazelProjectManager bazelProjectManager = ComponentContext.getInstance().getProjectManager();
BazelProject bazelProject = bazelProjectManager.getProject(projectName);
String testClassName = configuration.getAttribute("org.eclipse.jdt.launching.MAIN_TYPE", (String) null);
BazelProjectTargets targets = bazelProjectManager.getConfiguredBazelTargets(bazelProject, false);
File execRootDir = bazelWorkspace.getBazelExecRootDirectory();

// look for the param files for the test classname and/or targets
BazelJvmTestClasspathHelper.ParamFileResult testParamFilesResult = BazelJvmTestClasspathHelper
// each param file contains a Bazel specific list of data used when launching the test
BazelJvmTestClasspathHelper.ParamFileResult testParamFilesResult = bazelJvmTestClasspathHelper
.findParamFilesForTests(bazelWorkspace, bazelProject, isSource, testClassName, targets);

return computeUnresolvedClasspathFromParamFiles(execRootDir, testParamFilesResult);
}

/**
* Return the classpath entries needed to run the tests, using the Bazel param files for the
* targets as input
*/
IRuntimeClasspathEntry[] computeUnresolvedClasspathFromParamFiles(File execRootDir,
BazelJvmTestClasspathHelper.ParamFileResult testParamFilesResult) throws CoreException {
List<IRuntimeClasspathEntry> result = new ArrayList<>();

File base = bazelWorkspace.getBazelExecRootDirectory();
for (File paramsFile : testParamFilesResult.paramFiles) {
List<String> jarPaths;
try {
jarPaths = BazelJvmTestClasspathHelper.getClasspathJarsFromParamsFile(paramsFile);
} catch (IOException e) {
throw new CoreException(new Status(IStatus.ERROR, BUNDLE.getSymbolicName(),
"Error parsing " + paramsFile.getAbsolutePath(), e));
}
if (jarPaths == null) {
// error has already been logged, just try to soldier on
continue;
}
for (String rawPath : jarPaths) {
String canonicalPath = FSPathHelper.getCanonicalPathStringSafely(new File(base, rawPath));
IPath eachPath = new Path(canonicalPath);
if (eachPath.toFile().exists()) {
IRuntimeClasspathEntry src = JavaRuntime.newArchiveRuntimeClasspathEntry(eachPath);
result.add(src);
}
// assemble the de-duplicated list of jar files that are used across all the test targets
// these paths are listed in the param files, and are relative to the Bazel workspace exec root
Set<String> jarPaths = bazelJvmTestClasspathHelper.aggregateJarFilesFromParamFiles(testParamFilesResult);
for (String rawPath : jarPaths) {
String canonicalPath = FSPathHelper.getCanonicalPathStringSafely(new File(execRootDir, rawPath));
IPath eachPath = new Path(canonicalPath);
if (eachPath.toFile().exists()) {
IRuntimeClasspathEntry src = JavaRuntime.newArchiveRuntimeClasspathEntry(eachPath);
result.add(src);
}
}

// if there was a test target that had no param file, it is an unrunnable test
if (!testParamFilesResult.unrunnableLabels.isEmpty()) {
StringBuffer unrunnableLabelsString = new StringBuffer();
for (String label : testParamFilesResult.unrunnableLabels) {
unrunnableLabelsString.append(label);
unrunnableLabelsString.append(" ");
}
Display.getDefault().asyncExec(new Runnable() {
@Override
public void run() {
if (canOpenErrorDialog.get()) {
canOpenErrorDialog.set(false);

// only present the dialog once every ~10 times; often the user will
// be "yeah yeah, just run the tests that you can find" when this happens
// because if they run the tests over a project this can happen every time
// TODO create a pref for a user to be able to ignore these errors
javaTestDialogSkipCount++;
if (javaTestDialogSkipCount > 1) {
if (javaTestDialogSkipCount > 10) {
// trigger it next time
javaTestDialogSkipCount = 0;
}
return;
}

Display.getDefault().syncExec(new Runnable() {
@Override
public void run() {
MessageDialog.openError(Display.getDefault().getActiveShell(), "Unknown Target",
"One or more of the targets being executed are not part of a Bazel java_test target ( "
+ unrunnableLabelsString + "). The target(s) will be ignored.\n\n"
+ "Since this might be a common issue for your workspace, this dialog "
+ "will only be presented periodically when this happens.");
}
});
}
}
});
showUnrunnableErrorDialog(unrunnableLabelsString);
}

return result.toArray(new IRuntimeClasspathEntry[result.size()]);
}

/**
* Add the classpath providers to the configuration
*
Expand All @@ -237,4 +205,39 @@ public static void enable(ILaunchConfiguration config) throws CoreException {
wc.doSave();
}
}

private void showUnrunnableErrorDialog(StringBuffer unrunnableLabelsString) {
Display.getDefault().asyncExec(new Runnable() {
@Override
public void run() {
if (canOpenErrorDialog.get()) {
canOpenErrorDialog.set(false);

// only present the dialog once every ~10 times; often the user will
// be "yeah yeah, just run the tests that you can find" when this happens
// because if they run the tests over a project this can happen every time
// TODO create a pref for a user to be able to ignore these errors
javaTestDialogSkipCount++;
if (javaTestDialogSkipCount > 1) {
if (javaTestDialogSkipCount > 10) {
// trigger it next time
javaTestDialogSkipCount = 0;
}
return;
}

Display.getDefault().syncExec(new Runnable() {
@Override
public void run() {
MessageDialog.openError(Display.getDefault().getActiveShell(), "Unknown Target",
"One or more of the targets being executed are not part of a Bazel java_test target ( "
+ unrunnableLabelsString + "). The target(s) will be ignored.\n\n"
+ "Since this might be a common issue for your workspace, this dialog "
+ "will only be presented periodically when this happens.");
}
});
}
}
});
}
}
Loading

0 comments on commit bb0b0bc

Please sign in to comment.