Skip to content

Commit

Permalink
Run ng-killall in tearDown to limit memory taken by ng processes
Browse files Browse the repository at this point in the history
`Abnormal build process termination` in #133
was due to:

1. tests:all using rglobs is getting bigger and bigger
2. More integration tests are added, https://rbcommons.com/s/twitter/r/3819/
   added a few previously missed.

Now after running some pants commands, ng processes are taking close to the limit [1]
and eventually crashed later java process.

We have seen this behavior at Twitter in CI where memory is limited. Solution there
is to turn on ng only for compile, plus running ng-killall at the very beginning of
run (consists serveral subsequent pants test targets).

Here added is similar to that, to run ng-killall in `tearDown` as cleanup, at a cost of
increasing the overall test time, but that hopefully we can address via
#137. Turning off ng is another
option, but would rather test more real settings for integration tests.

[1] "3GB of memory and up to 2 cores" https://travis-ci.com/plans

Testing Done:
https://travis-ci.org/peiyuwang/intellij-pants-plugin/builds/128219704 passed
https://travis-ci.org/peiyuwang/intellij-pants-plugin/builds/128466370 passed (calling passed whenever I see the first 3 shards succeeded)

Bugs closed: 133, 138

Reviewed at https://rbcommons.com/s/twitter/r/3831/
  • Loading branch information
peiyuwang authored and Peiyu Wang committed May 8, 2016
1 parent 655ad4a commit 9a1ef40
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 8 deletions.
5 changes: 5 additions & 0 deletions common/com/twitter/intellij/pants/util/PantsUtil.java
Expand Up @@ -261,6 +261,11 @@ public static VirtualFile findProjectManifestJar(@NotNull Project myProject) {
return null;
}

public static GeneralCommandLine defaultCommandLine(@NotNull Project project) throws PantsException {
VirtualFile pantsExecutable = PantsUtil.findPantsExecutable(project);
return defaultCommandLine(pantsExecutable.getPath());
}

public static GeneralCommandLine defaultCommandLine(@NotNull String projectPath) throws PantsException {
final File pantsExecutable = PantsUtil.findPantsExecutable(new File(projectPath));
if (pantsExecutable == null) {
Expand Down
Expand Up @@ -3,6 +3,7 @@

package com.twitter.intellij.pants.testFramework;

import com.google.common.base.Joiner;
import com.intellij.compiler.impl.ModuleCompileScope;
import com.intellij.execution.ExecutionException;
import com.intellij.execution.ProgramRunnerUtil;
Expand Down Expand Up @@ -161,6 +162,15 @@ private void cmd(String... args) throws ExecutionException {
assertTrue("Failed to execute: " + StringUtil.join(args, " "), cmdOutput.getExitCode() == 0);
}

protected void killNailgun() throws ExecutionException {
// NB: the ideal interface here is defaultCommandLine(myProject). However,
// not all tests call doImport therefore myProject may not always contain modules.
final GeneralCommandLine commandLine = PantsUtil.defaultCommandLine(getProjectPath());
commandLine.addParameter("ng-killall");
// Wait for command to finish.
PantsUtil.getCmdOutput(commandLine, null);
}

@NotNull
abstract protected File getProjectFolder();

Expand Down Expand Up @@ -339,7 +349,9 @@ private List<String> assertCompilerMessages(List<CompilerMessage> messages) {
);
switch (message.getCategory()) {
case ERROR:
fail("Compilation failed with error: " + prettyMessage);
// Always show full error messages.

fail("Compilation failed with error: " + Joiner.on('\n').join(messages));
break;
case WARNING:
System.out.println("Compilation warning: " + prettyMessage);
Expand Down Expand Up @@ -488,6 +500,9 @@ public void tearDown() throws Exception {
if (myCompilerTester != null) {
myCompilerTester.tearDown();
}

// Kill nailgun after usage as memory on travis is limited, at a cost of slower later builds.
killNailgun();
cleanProjectRoot();
Messages.setTestDialog(TestDialog.DEFAULT);
}
Expand Down
Expand Up @@ -6,13 +6,7 @@
import com.twitter.intellij.pants.testFramework.OSSPantsIntegrationTest;

public class OSSPantsExamplesMultiTargetsIntegrationTest extends OSSPantsIntegrationTest {
public void testEmpty() {
// Unfortunately this is in junit 3 we need at least one method starts with 'test'.
// Remove once https://github.com/pantsbuild/intellij-pants-plugin/issues/133 is addressed.
}

// TODO (peiyu) https://github.com/pantsbuild/intellij-pants-plugin/issues/133
public void IGNORE_testHello() throws Throwable {
public void testHello() throws Throwable {
doImport("examples/src/java/org/pantsbuild/example/hello");

assertModules(
Expand Down

0 comments on commit 9a1ef40

Please sign in to comment.