Various fixes accumulated so far (#2229)

* Various fixes accumulated so far
* Revert SnakeYAML version change in feature.xml
* Resolve merge conflict due to parallel edit

Author: chanseokoh

plugins/com.google.cloud.tools.eclipse.appengine.deploy.ui.test/src/com/google/cloud/tools/eclipse/appengine/deploy/ui/GcpProjectQueryJobTest.java

@@ -29,6 +29,7 @@
 import com.google.cloud.tools.eclipse.projectselector.ProjectSelector;
 import com.google.cloud.tools.eclipse.projectselector.model.GcpProject;
 import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import java.util.List;
 import org.eclipse.core.databinding.DataBindingContext;
 import org.eclipse.core.runtime.jobs.Job;
@@ -121,13 +122,11 @@ public void testRun_abandonStaleJob() throws InterruptedException, ProjectReposi
     ProjectRepository projectRepository2 = mock(ProjectRepository.class);
     when(projectRepository2.getProjects(staleCredential)).thenReturn(anotherProjectList);
 
-    Predicate<Job> notLatest = mock(Predicate.class);
+    // This second job is stale, i.e., it was fired, but user has selected another credential.
+    Predicate<Job> notLatest = Predicates.alwaysFalse();
     Job staleJob = new GcpProjectQueryJob(staleCredential, projectRepository2,
         projectSelector, dataBindingContext, notLatest);
 
-    // This second job is stale, i.e., it was fired, but user has selected another credential.
-    when(notLatest.apply(staleJob)).thenReturn(false);
-
     queryJob.schedule();
     queryJob.join();
     // Make the stale job complete even after "queryJob" finishes.

plugins/com.google.cloud.tools.eclipse.appengine.deploy.ui.test/src/com/google/cloud/tools/eclipse/appengine/deploy/ui/internal/AppEngineApplicationQueryJobTest.java

@@ -33,6 +33,7 @@
 import com.google.cloud.tools.eclipse.projectselector.model.AppEngine;
 import com.google.cloud.tools.eclipse.projectselector.model.GcpProject;
 import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.jface.viewers.IStructuredSelection;
 import org.eclipse.swt.widgets.Display;
@@ -186,13 +187,11 @@ public void testRun_abandonStaleJob() throws InterruptedException, ProjectReposi
     when(projectRepository2.getAppEngineApplication(staleCredential, "staleProjectId"))
         .thenThrow(new ProjectRepositoryException("testException"));
 
-    Predicate<Job> notLatest = mock(Predicate.class);
+    // This second job is stale, i.e., it was fired, but user has selected another credential.
+    Predicate<Job> notLatest = Predicates.alwaysFalse();
     Job staleJob = new AppEngineApplicationQueryJob(staleProject, staleCredential,
         projectRepository2, projectSelector, EXPECTED_LINK, notLatest);
 
-    // This second job is stale, i.e., it was fired, but user has selected another credential.
-    when(notLatest.apply(staleJob)).thenReturn(false);
-
     queryJob.schedule();
     queryJob.join();
     // Make the stale job complete even after "queryJob" finishes.

plugins/com.google.cloud.tools.eclipse.appengine.deploy.ui/src/com/google/cloud/tools/eclipse/appengine/deploy/ui/flexible/FlexDeployPreferencesPanel.java

@@ -28,9 +28,9 @@
 import org.eclipse.core.resources.IProject;
 import org.eclipse.jface.databinding.swt.ISWTObservableValue;
 import org.eclipse.jface.databinding.swt.WidgetProperties;
+import org.eclipse.jface.layout.GridDataFactory;
 import org.eclipse.jface.layout.GridLayoutFactory;
 import org.eclipse.swt.SWT;
-import org.eclipse.swt.layout.GridData;
 import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Label;
@@ -72,10 +72,8 @@ private void createAppYamlSection() {
     browse.setText(Messages.getString("deploy.preferences.dialog.browse"));
     browse.addSelectionListener(new RelativeFileFieldSetter(appYamlField, project.getLocation()));
 
-    GridLayoutFactory.fillDefaults().numColumns(2).applyTo(secondColumn);
-    GridData fillGridData = new GridData(SWT.FILL, SWT.CENTER, true, false);
-    secondColumn.setLayoutData(fillGridData);
-    appYamlField.setLayoutData(fillGridData);
+    GridLayoutFactory.fillDefaults().numColumns(2).generateLayout(secondColumn);
+    GridDataFactory.fillDefaults().applyTo(secondColumn);
 
     // Part 2. set up data binding
     ISWTObservableValue fieldValue = WidgetProperties.text(SWT.Modify).observe(appYamlField);

plugins/com.google.cloud.tools.eclipse.appengine.libraries.test/src/com/google/cloud/tools/eclipse/appengine/libraries/LibraryClasspathContainerTest.java

@@ -45,6 +45,7 @@ public void setUp() {
                                                        mockClasspathEntry);
   }
 
+  @Test
   public void testConstructor_nullPath() {
     try {
       new LibraryClasspathContainer(null, "description", mockClasspathEntry);
@@ -54,6 +55,7 @@ public void testConstructor_nullPath() {
     }
   }
 
+  @Test
   public void testConstructor_nullDescription() {
     try {
       new LibraryClasspathContainer(new Path("container/path"), null, mockClasspathEntry);
@@ -63,15 +65,17 @@ public void testConstructor_nullDescription() {
     }
   }
 
+  @Test
   public void testConstructor_emptyDescription() {
     try {
       new LibraryClasspathContainer(new Path("container/path"), "", mockClasspathEntry);
-      fail("Expected NullPointerException");
-    } catch (NullPointerException ex) {
+      fail("Expected IllegalArgumentException");
+    } catch (IllegalArgumentException ex) {
       assertNotNull(ex.getMessage());
     }
   }
 
+  @Test
   public void testConstructor_nullClasspathEntries() {
     try {
       new LibraryClasspathContainer(new Path("container/path"), "description", null);
@@ -92,7 +96,7 @@ public void testGetDescription() {
   }
 
   @Test
-  public void testGetKind_returnsApplication() throws Exception {
+  public void testGetKind_returnsApplication() {
     assertThat(classpathContainer.getKind(), is(IClasspathContainer.K_APPLICATION));
   }
 

plugins/com.google.cloud.tools.eclipse.appengine.libraries.test/src/com/google/cloud/tools/eclipse/appengine/libraries/model/LibraryFactoryTest.java

@@ -19,13 +19,12 @@
 import org.eclipse.core.runtime.IConfigurationElement;
 import org.junit.Assert;
 import org.junit.Test;
-import org.junit.runner.RunWith;
 import org.mockito.Mockito;
 
 public class LibraryFactoryTest {
-  
+
   private LibraryFactory factory = new LibraryFactory();
-  
+
   @Test
   public void testCreate() throws LibraryFactoryException {
     IConfigurationElement configuration = Mockito.mock(IConfigurationElement.class);
@@ -43,16 +42,16 @@ public void testCreate() throws LibraryFactoryException {
     Library library = factory.create(configuration);
     Assert.assertEquals("com.google.guava", library.getGroup());
   }
-  
+
   @Test
   public void testCreate_null() throws LibraryFactoryException {
     try {
       factory.create(null);
       Assert.fail();
     } catch (NullPointerException ex) {
-    } 
+    }
   }
-  
+
   @Test
   public void testCreate_nonLibrary() {
     IConfigurationElement configuration = Mockito.mock(IConfigurationElement.class);
@@ -61,7 +60,7 @@ public void testCreate_nonLibrary() {
       Assert.fail();
     } catch (LibraryFactoryException ex) {
       Assert.assertNotNull(ex.getMessage());
-    } 
+    }
   }
-  
+
 }

plugins/com.google.cloud.tools.eclipse.appengine.libraries.test/src/com/google/cloud/tools/eclipse/appengine/libraries/persistence/LibraryClasspathContainerSerializerTest.java

@@ -90,7 +90,7 @@ public class LibraryClasspathContainerSerializerTest {
   private LibraryClasspathContainer container;
 
   @Before
-  public void setUp() throws Exception {
+  public void setUp() {
     List<IClasspathEntry> classpathEntries = Arrays.asList(
         newClasspathEntry(IClasspathEntry.CPE_LIBRARY, "/test/path/to/jar",
             "/test/path/to/src", new IClasspathAttribute[] {newAttribute("attrName", "attrValue")},

plugins/com.google.cloud.tools.eclipse.appengine.libraries.ui.test/src/com/google/cloud/tools/eclipse/appengine/libraries/ui/AppEngineLibrariesSelectorGroupTest.java

@@ -52,7 +52,7 @@ public class AppEngineLibrariesSelectorGroupTest {
   private SWTBotCheckBox objectifyButton;
 
   @Before
-  public void setUp() throws Exception {
+  public void setUp() {
     shell = shellTestResource.getShell();
     shell.setLayout(new FillLayout());
     librariesSelector = new LibrarySelectorGroup(shell, CloudLibraries.APP_ENGINE_GROUP);

plugins/com.google.cloud.tools.eclipse.appengine.localserver.test/src/com/google/cloud/tools/eclipse/appengine/localserver/server/LocalAppEngineServerLaunchConfigurationDelegateTest.java

@@ -61,7 +61,7 @@ public class LocalAppEngineServerLaunchConfigurationDelegateTest {
   private LocalAppEngineServerBehaviour serverBehavior;
 
   @Before
-  public void setUp() throws CoreException {
+  public void setUp() {
     when(server.loadAdapter(any(Class.class), any(IProgressMonitor.class)))
         .thenReturn(serverBehavior);
   }

plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/server/LocalAppEngineServerLaunchConfigurationDelegate.java

@@ -590,10 +590,10 @@ private static IProject[] getReferencedProjects(ILaunchConfiguration configurati
    * </ul>
    */
   private class CloudSdkDebugTarget extends DebugElement implements IDebugTarget {
-    private ILaunch launch;
-    private LocalAppEngineServerBehaviour serverBehaviour;
-    private IServer server;
-    private IOConsole console;
+    private final ILaunch launch;
+    private final LocalAppEngineServerBehaviour serverBehaviour;
+    private final IServer server;
+    private final IOConsole console;
 
     // Fire a {@link DebugEvent#TERMINATED} event when the server is stopped
     private IServerListener serverEventsListener = new IServerListener() {
@@ -655,7 +655,7 @@ public void launchesRemoved(ILaunch[] launches) {
       }
     };
 
-    CloudSdkDebugTarget(ILaunch launch, LocalAppEngineServerBehaviour serverBehaviour,
+    private CloudSdkDebugTarget(ILaunch launch, LocalAppEngineServerBehaviour serverBehaviour,
         IOConsole console) {
       super(null);
       this.launch = launch;
@@ -664,14 +664,13 @@ public void launchesRemoved(ILaunch[] launches) {
       this.console = console;
     }
 
-    protected void removeConsole() {
+    private void removeConsole() {
       ConsolePlugin plugin = ConsolePlugin.getDefault();
       IConsoleManager manager = plugin.getConsoleManager();
       manager.removeConsoles(new IConsole[] {console});
       console.destroy();
     }
 
-
     /** Add required listeners */
     private void engage() {
       getLaunchManager().addLaunchListener(launchesListener);

plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/ui/LocalAppEngineConsole.java

@@ -33,9 +33,9 @@
  * A console that displays information for a run/debug session of the App Engine runtime
  */
 public class LocalAppEngineConsole extends MessageConsole {
-  private LocalAppEngineServerBehaviour serverBehaviour;
-  private String unprefixedName;
-  private IServerListener serverStateListener = new IServerListener() {
+  private final LocalAppEngineServerBehaviour serverBehaviour;
+  private final String unprefixedName;
+  private final IServerListener serverStateListener = new IServerListener() {
     @Override
     public void serverChanged(ServerEvent event) {
       if ((event.getKind() & ServerEvent.SERVER_CHANGE) != 0) {
@@ -50,7 +50,7 @@ private LocalAppEngineConsole(String name, LocalAppEngineServerBehaviour serverB
     this.serverBehaviour = serverBehaviour;
   }
 
-  protected void update(int serverState) {
+  private void update(int serverState) {
     if (serverState == IServer.STATE_STOPPED) {
       disengage(); // we should no longer update
     }
@@ -60,17 +60,14 @@ protected void update(int serverState) {
   /**
    * Update the shown name with the server stop/stopping state.
    */
-  protected void updateName(int serverState) {
+  private void updateName(int serverState) {
     final String computedName;
     if (serverState == IServer.STATE_STARTING) {
-      computedName =
-          Messages.getString("SERVER_STARTING_TEMPLATE", unprefixedName);
+      computedName = Messages.getString("SERVER_STARTING_TEMPLATE", unprefixedName);
     } else if (serverState == IServer.STATE_STOPPING) {
-      computedName =
-          Messages.getString("SERVER_STOPPING_TEMPLATE", unprefixedName);
+      computedName = Messages.getString("SERVER_STOPPING_TEMPLATE", unprefixedName);
     } else if (serverState == IServer.STATE_STOPPED) {
-      computedName =
-          Messages.getString("SERVER_STOPPED_TEMPLATE", unprefixedName);
+      computedName = Messages.getString("SERVER_STOPPED_TEMPLATE", unprefixedName);
     } else {
       computedName = unprefixedName;
     }
@@ -110,7 +107,7 @@ private void disengage() {
 
   public static class Factory implements ConsoleFactory<LocalAppEngineConsole> {
 
-    private LocalAppEngineServerBehaviour serverBehaviour;
+    private final LocalAppEngineServerBehaviour serverBehaviour;
 
     public Factory(LocalAppEngineServerBehaviour serverBehaviour) {
       this.serverBehaviour = serverBehaviour;