From 924f62d219770083cc52773c5bb9cbacfd931966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20=C5=BBarnowski?= Date: Thu, 9 Apr 2020 00:50:52 +0200 Subject: [PATCH] Use user data to determine if debug is enabled (#515) --- .../pants/service/task/PantsTaskManager.java | 43 +++++++++---------- ...ntsJvmRunConfigurationIntegrationTest.java | 33 ++++++++------ .../service/task/PantsTaskManagerTest.java | 14 ++++-- 3 files changed, 52 insertions(+), 38 deletions(-) diff --git a/src/com/twitter/intellij/pants/service/task/PantsTaskManager.java b/src/com/twitter/intellij/pants/service/task/PantsTaskManager.java index 0ed3f8e29..af750262d 100644 --- a/src/com/twitter/intellij/pants/service/task/PantsTaskManager.java +++ b/src/com/twitter/intellij/pants/service/task/PantsTaskManager.java @@ -28,12 +28,12 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; +import java.util.Optional; -import static com.intellij.openapi.externalSystem.rt.execution.ForkedDebuggerHelper.DEBUG_FORK_SOCKET_PARAM; +import static com.intellij.openapi.externalSystem.rt.execution.ForkedDebuggerHelper.JVM_DEBUG_SETUP_PREFIX; +import static com.intellij.openapi.externalSystem.service.execution.ExternalSystemRunnableState.BUILD_PROCESS_DEBUGGER_PORT_KEY; public class PantsTaskManager implements ExternalSystemTaskManager { @@ -58,9 +58,7 @@ public void executeTasks( return; } - final GeneralCommandLine commandLine = - constructCommandLine( - taskNames, projectPath, settings, Lists.newArrayList(settings.getVmOptions()), settings.getArguments(), jvmAgentSetup); + GeneralCommandLine commandLine = constructCommandLine(taskNames, projectPath, settings); if (commandLine == null) { return; } @@ -104,23 +102,19 @@ public void onTextAvailable(ProcessEvent event, Key outputType) { } } - /** - * Eliminate `DEBUG_FORK_SOCKET_PARAM`(-forkSocket) from debug setup parameters - */ @VisibleForTesting - protected static String getCleanedDebugSetup(String debugSetup) { - ArrayList params = Lists.newArrayList(debugSetup.split(" ")); - return params.stream().filter(s -> !s.contains(DEBUG_FORK_SOCKET_PARAM)).collect(Collectors.joining(" ")); + protected static void setupDebuggerSettings(PantsExecutionSettings settings) { + Optional.ofNullable(settings.getUserData(BUILD_PROCESS_DEBUGGER_PORT_KEY)) + .filter(port -> port > 0) + .map(port -> JVM_DEBUG_SETUP_PREFIX + port) + .ifPresent(settings::withVmOption); } @Nullable public GeneralCommandLine constructCommandLine( @NotNull List taskNames, @NotNull String projectPath, - @Nullable PantsExecutionSettings settings, - @NotNull List vmOptions, - @NotNull List scriptParameters, - @Nullable String debuggerSetup + @Nullable PantsExecutionSettings settings ) { if (settings == null) { return null; @@ -128,20 +122,23 @@ public GeneralCommandLine constructCommandLine( projectPath = PantsTargetAddress.extractPath(projectPath).get(); final GeneralCommandLine commandLine = PantsUtil.defaultCommandLine(projectPath); + setupDebuggerSettings(settings); + boolean debugEnabled = settings.getJvmArguments().stream() + .anyMatch(jvmOpt -> jvmOpt.startsWith(JVM_DEBUG_SETUP_PREFIX)); + /** * Global options section. */ - if (debuggerSetup != null) { + if (debugEnabled) { if (taskNames.size() > 1) { throw new ExternalSystemException(PantsBundle.message("pants.error.multiple.tasks.for.debugging")); } - commandLine.addParameter(PantsConstants.PANTS_CLI_OPTION_NO_TEST_JUNIT_TIMEOUTS); final String goal = taskNames.iterator().next(); - final String jvmOptionsFlag = goal2JvmOptionsFlag.get(goal); - if (jvmOptionsFlag == null) { + if (!goal2JvmOptionsFlag.containsKey(goal)) { throw new ExternalSystemException(PantsBundle.message("pants.error.cannot.debug.task", goal)); } - commandLine.addParameter(jvmOptionsFlag + "=" + getCleanedDebugSetup(debuggerSetup)); + + commandLine.addParameter(PantsConstants.PANTS_CLI_OPTION_NO_TEST_JUNIT_TIMEOUTS); } if (settings.isUseIdeaProjectJdk()) { try { @@ -161,14 +158,14 @@ public GeneralCommandLine constructCommandLine( if (jvmOptionsFlag == null) { continue; } - for (String vmOption : vmOptions) { + for (String vmOption : settings.getJvmArguments()) { commandLine.addParameter(jvmOptionsFlag + "=" + vmOption); } } /** * Script parameters section including targets and options. */ - commandLine.addParameters(scriptParameters); + commandLine.addParameters(settings.getArguments()); return commandLine; } diff --git a/tests/com/twitter/intellij/pants/execution/OSSPantsJvmRunConfigurationIntegrationTest.java b/tests/com/twitter/intellij/pants/execution/OSSPantsJvmRunConfigurationIntegrationTest.java index b1e5ecef8..ba11f1b95 100644 --- a/tests/com/twitter/intellij/pants/execution/OSSPantsJvmRunConfigurationIntegrationTest.java +++ b/tests/com/twitter/intellij/pants/execution/OSSPantsJvmRunConfigurationIntegrationTest.java @@ -22,11 +22,11 @@ import com.intellij.psi.PsiPackage; import com.intellij.testFramework.MapDataContext; import com.twitter.intellij.pants.PantsManager; -import com.twitter.intellij.pants.util.ProjectTestJvms; import com.twitter.intellij.pants.service.task.PantsTaskManager; import com.twitter.intellij.pants.settings.PantsExecutionSettings; import com.twitter.intellij.pants.testFramework.OSSPantsIntegrationTest; import com.twitter.intellij.pants.util.PantsUtil; +import com.twitter.intellij.pants.util.ProjectTestJvms; import org.jetbrains.annotations.NotNull; import org.junit.Assert; @@ -36,10 +36,14 @@ import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.OptionalInt; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.intellij.openapi.externalSystem.rt.execution.ForkedDebuggerHelper.JVM_DEBUG_SETUP_PREFIX; +import static com.intellij.openapi.externalSystem.service.execution.ExternalSystemRunnableState.BUILD_PROCESS_DEBUGGER_PORT_KEY; + public class OSSPantsJvmRunConfigurationIntegrationTest extends OSSPantsIntegrationTest { public void testClassRunConfiguration() throws Throwable { @@ -67,14 +71,17 @@ public void testClassRunConfiguration() throws Throwable { ExternalSystemManager.EP_NAME.findExtension(PantsManager.class).getTaskManagerClass(); assertEquals(PantsTaskManager.class, taskManagerClass); - String debuggerSetup = "dummy_debugger_setup"; - - List debugParameters = Arrays.asList("--no-test-junit-timeouts", "--jvm-test-junit-options=" + debuggerSetup, "test"); + int debugPort = 5005; + List debugParameters = Arrays.asList( + "--no-test-junit-timeouts", + "test", + "--jvm-test-junit-options=" + JVM_DEBUG_SETUP_PREFIX + debugPort + ); List expectedDebugParameters = Stream.of(debugParameters, configScriptParameters) .flatMap(Collection::stream) .collect(Collectors.toList()); - GeneralCommandLine finalDebugCommandline = getFinalCommandline(esc, debuggerSetup, taskManagerClass); + GeneralCommandLine finalDebugCommandline = getFinalCommandline(esc, taskManagerClass, OptionalInt.of(debugPort)); assertEquals(expectedDebugParameters, finalDebugCommandline.getParametersList().getParameters()); List runParameters = Collections.singletonList("test"); @@ -82,23 +89,25 @@ public void testClassRunConfiguration() throws Throwable { .flatMap(Collection::stream) .collect(Collectors.toList()); - GeneralCommandLine finalRunCommandline = getFinalCommandline(esc, null, taskManagerClass); + GeneralCommandLine finalRunCommandline = getFinalCommandline(esc, taskManagerClass, OptionalInt.empty()); assertEquals(expectedRunParameters, finalRunCommandline.getParametersList().getParameters()); } @NotNull private GeneralCommandLine getFinalCommandline( ExternalSystemRunConfiguration esc, - String debuggerSetup, - Class> taskManagerClass + Class> taskManagerClass, + OptionalInt debugPort ) throws InstantiationException, IllegalAccessException { + PantsExecutionSettings settings = PantsExecutionSettings.createDefault(); + settings.withVmOptions(PantsUtil.parseCmdParameters(Optional.ofNullable(esc.getSettings().getVmOptions()))); + settings.withArguments(PantsUtil.parseCmdParameters(Optional.ofNullable(esc.getSettings().getScriptParameters()))); + debugPort.ifPresent(port -> settings.putUserData(BUILD_PROCESS_DEBUGGER_PORT_KEY, port)); + GeneralCommandLine commandLine = ((PantsTaskManager) taskManagerClass.newInstance()).constructCommandLine( esc.getSettings().getTaskNames(), esc.getSettings().getExternalProjectPath(), - PantsExecutionSettings.createDefault(), - PantsUtil.parseCmdParameters(Optional.ofNullable(esc.getSettings().getVmOptions())), - PantsUtil.parseCmdParameters(Optional.ofNullable(esc.getSettings().getScriptParameters())), - debuggerSetup + settings ); assertNotNull(commandLine); return commandLine; diff --git a/tests/com/twitter/intellij/pants/service/task/PantsTaskManagerTest.java b/tests/com/twitter/intellij/pants/service/task/PantsTaskManagerTest.java index e144769a6..67b15e9c6 100644 --- a/tests/com/twitter/intellij/pants/service/task/PantsTaskManagerTest.java +++ b/tests/com/twitter/intellij/pants/service/task/PantsTaskManagerTest.java @@ -3,13 +3,21 @@ package com.twitter.intellij.pants.service.task; +import com.twitter.intellij.pants.settings.PantsExecutionSettings; import junit.framework.TestCase; +import java.util.Collections; + +import static com.intellij.openapi.externalSystem.service.execution.ExternalSystemRunnableState.BUILD_PROCESS_DEBUGGER_PORT_KEY; + public class PantsTaskManagerTest extends TestCase { public void testGetCleanedDebugSetup() { - String cleanedSetup = - PantsTaskManager.getCleanedDebugSetup("-agentlib:jdwp=transport=dt_socket,server=n,suspend=y,address=63212 -forkSocket63213"); - assertEquals("-agentlib:jdwp=transport=dt_socket,server=n,suspend=y,address=63212", cleanedSetup); + PantsExecutionSettings settings = new PantsExecutionSettings(Collections.emptyList(), false, false, false, false, false); + settings.putUserData(BUILD_PROCESS_DEBUGGER_PORT_KEY, 63212); + + PantsTaskManager.setupDebuggerSettings(settings); + assertEquals(1, settings.getJvmArguments().size()); + assertEquals("-agentlib:jdwp=transport=dt_socket,server=n,suspend=y,address=63212", settings.getJvmArguments().get(0)); } }