You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@continuum.apache.org by ba...@apache.org on 2015/05/06 00:44:38 UTC

svn commit: r1677905 - in /continuum/trunk: continuum-api/src/main/java/org/apache/continuum/dao/ continuum-core/src/main/java/org/apache/maven/continuum/ continuum-core/src/test/java/org/apache/maven/continuum/ continuum-store/src/main/java/org/apache...

Author: batkinson
Date: Tue May  5 22:44:38 2015
New Revision: 1677905

URL: http://svn.apache.org/r1677905
Log:
[CONTINUUM-2765] Changes since last success appears to always compute no changes

Modified:
    continuum/trunk/continuum-api/src/main/java/org/apache/continuum/dao/BuildResultDao.java
    continuum/trunk/continuum-core/src/main/java/org/apache/maven/continuum/DefaultContinuum.java
    continuum/trunk/continuum-core/src/test/java/org/apache/maven/continuum/DefaultContinuumTest.java
    continuum/trunk/continuum-store/src/main/java/org/apache/continuum/dao/BuildResultDaoImpl.java
    continuum/trunk/continuum-webapp/src/main/resources/localization/Continuum.properties

Modified: continuum/trunk/continuum-api/src/main/java/org/apache/continuum/dao/BuildResultDao.java
URL: http://svn.apache.org/viewvc/continuum/trunk/continuum-api/src/main/java/org/apache/continuum/dao/BuildResultDao.java?rev=1677905&r1=1677904&r2=1677905&view=diff
==============================================================================
--- continuum/trunk/continuum-api/src/main/java/org/apache/continuum/dao/BuildResultDao.java (original)
+++ continuum/trunk/continuum-api/src/main/java/org/apache/continuum/dao/BuildResultDao.java Tue May  5 22:44:38 2015
@@ -55,8 +55,7 @@ public interface BuildResultDao
 
     BuildResult getLatestBuildResultInSuccess( int projectId );
 
-    BuildResult getPreviousBuildResultInSuccess( int projectId, int buildResultId )
-        throws ContinuumStoreException;
+    BuildResult getPreviousBuildResultInSuccess( int projectId, int buildResultId );
 
     /**
      * Marks results in the BUILDING status with a start time before the specified cutoff as CANCELLED.

Modified: continuum/trunk/continuum-core/src/main/java/org/apache/maven/continuum/DefaultContinuum.java
URL: http://svn.apache.org/viewvc/continuum/trunk/continuum-core/src/main/java/org/apache/maven/continuum/DefaultContinuum.java?rev=1677905&r1=1677904&r2=1677905&view=diff
==============================================================================
--- continuum/trunk/continuum-core/src/main/java/org/apache/maven/continuum/DefaultContinuum.java (original)
+++ continuum/trunk/continuum-core/src/main/java/org/apache/maven/continuum/DefaultContinuum.java Tue May  5 22:44:38 2015
@@ -111,7 +111,15 @@ import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.PrintWriter;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -1135,78 +1143,30 @@ public class DefaultContinuum
     public List<ChangeSet> getChangesSinceLastSuccess( int projectId, int buildResultId )
         throws ContinuumException
     {
-        BuildResult previousBuildResult = null;
-        try
-        {
-            previousBuildResult = buildResultDao.getPreviousBuildResultInSuccess( projectId, buildResultId );
-        }
-        catch ( ContinuumStoreException e )
-        {
-            //No previous build in success, Nothing to do
-        }
-        int lastSuccessId = previousBuildResult == null ? 0 : previousBuildResult.getId();
-        ArrayList<BuildResult> buildResults = new ArrayList<BuildResult>(
-            buildResultDao.getBuildResultsForProjectWithDetails( projectId, lastSuccessId, buildResultId ) );
-
-        Iterator<BuildResult> buildResultsIterator = buildResults.iterator();
-
-        boolean stop = false;
+        List<ChangeSet> changes = new ArrayList<ChangeSet>();
 
-        //TODO: Shouldn't be used now with the previous call of buildResultDao.getBuildResultsForProjectWithDetails
-        while ( !stop )
+        /*
+           Assumption: users will not find changes between project addition and first success very useful.
+           This also prevents inadvertently computing huge change lists during exceptional conditions by defaulting
+           to first id.
+         */
+        BuildResult previousSuccess = buildResultDao.getPreviousBuildResultInSuccess( projectId, buildResultId );
+        if ( previousSuccess != null )
         {
-            if ( buildResultsIterator.hasNext() )
-            {
-                BuildResult buildResult = buildResultsIterator.next();
+            List<BuildResult> resultsSinceLastSuccess = buildResultDao.getBuildResultsForProjectWithDetails(
+                projectId, previousSuccess.getId(), buildResultId );
 
-                if ( buildResult.getId() == buildResultId )
+            for ( BuildResult result : resultsSinceLastSuccess )
+            {
+                ScmResult scmResult = result.getScmResult();
+                if ( scmResult != null )
                 {
-                    stop = true;
+                    changes.addAll( scmResult.getChanges() );
                 }
             }
-            else
-            {
-                stop = true;
-            }
         }
 
-        if ( !buildResultsIterator.hasNext() )
-        {
-            return null;
-        }
-
-        BuildResult buildResult = buildResultsIterator.next();
-
-        List<ChangeSet> changes = null;
-
-        while ( buildResult.getState() != ContinuumProjectState.OK )
-        {
-            if ( changes == null )
-            {
-                changes = new ArrayList<ChangeSet>();
-            }
-
-            ScmResult scmResult = buildResult.getScmResult();
-
-            if ( scmResult != null )
-            {
-                changes.addAll( scmResult.getChanges() );
-            }
-
-            if ( !buildResultsIterator.hasNext() )
-            {
-                return changes;
-            }
-
-            buildResult = buildResultsIterator.next();
-        }
-
-        if ( changes == null )
-        {
-            changes = Collections.EMPTY_LIST;
-        }
-
-        return changes;
+        return changes.isEmpty() ? Collections.EMPTY_LIST : changes;
     }
 
     // ----------------------------------------------------------------------
@@ -3952,6 +3912,10 @@ public class DefaultContinuum
         this.projectDao = projectDao;
     }
 
+    void setBuildResultDao( BuildResultDao buildResultDao ) {
+        this.buildResultDao = buildResultDao;
+    }
+
     public DistributedBuildManager getDistributedBuildManager()
     {
         return distributedBuildManager;

Modified: continuum/trunk/continuum-core/src/test/java/org/apache/maven/continuum/DefaultContinuumTest.java
URL: http://svn.apache.org/viewvc/continuum/trunk/continuum-core/src/test/java/org/apache/maven/continuum/DefaultContinuumTest.java?rev=1677905&r1=1677904&r2=1677905&view=diff
==============================================================================
--- continuum/trunk/continuum-core/src/test/java/org/apache/maven/continuum/DefaultContinuumTest.java (original)
+++ continuum/trunk/continuum-core/src/test/java/org/apache/maven/continuum/DefaultContinuumTest.java Tue May  5 22:44:38 2015
@@ -19,6 +19,7 @@ package org.apache.maven.continuum;
  * under the License.
  */
 
+import edu.emory.mathcs.backport.java.util.Arrays;
 import org.apache.continuum.buildmanager.BuildsManager;
 import org.apache.continuum.dao.BuildResultDao;
 import org.apache.continuum.dao.ProjectDao;
@@ -37,6 +38,8 @@ import org.apache.maven.continuum.model.
 import org.apache.maven.continuum.model.project.Project;
 import org.apache.maven.continuum.model.project.ProjectGroup;
 import org.apache.maven.continuum.model.project.ProjectNotifier;
+import org.apache.maven.continuum.model.scm.ChangeSet;
+import org.apache.maven.continuum.model.scm.ScmResult;
 import org.apache.maven.continuum.project.builder.ContinuumProjectBuildingResult;
 import org.apache.maven.shared.release.ReleaseResult;
 import org.junit.Before;
@@ -47,11 +50,13 @@ import org.slf4j.LoggerFactory;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 import static org.junit.Assert.*;
+import static org.mockito.Matchers.anyInt;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -75,6 +80,7 @@ public class DefaultContinuumTest
     {
         taskQueueManager = mock( TaskQueueManager.class );
         projectDao = mock( ProjectDao.class );
+        buildResultDao = mock( BuildResultDao.class );
     }
 
     @Test
@@ -746,10 +752,98 @@ public class DefaultContinuumTest
         }
     }
 
-    private Continuum getContinuum()
+    @Test
+    public void testGetChangesSinceLastSuccessNoSuccess()
+        throws Exception
+    {
+        DefaultContinuum continuum = getContinuum();
+        continuum.setBuildResultDao( buildResultDao );
+
+        when( buildResultDao.getPreviousBuildResultInSuccess( anyInt(), anyInt() ) ).thenReturn( null );
+
+        List<ChangeSet> changes = continuum.getChangesSinceLastSuccess( 5, 5 );
+
+        assertEquals( "no prior success should return no changes", 0, changes.size() );
+    }
+
+    @Test
+    public void testGetChangesSinceLastSuccessNoInterveningFailures()
+        throws Exception
+    {
+        DefaultContinuum continuum = getContinuum();
+        continuum.setBuildResultDao( buildResultDao );
+
+        int projectId = 123, fromId = 789, toId = 1011;
+        BuildResult priorResult = new BuildResult();
+        priorResult.setId( fromId );
+
+        when( buildResultDao.getPreviousBuildResultInSuccess( projectId, toId ) ).thenReturn( priorResult );
+        when( buildResultDao.getBuildResultsForProjectWithDetails( projectId, fromId, toId ) ).thenReturn(
+            Collections.EMPTY_LIST );
+
+        List<ChangeSet> changes = continuum.getChangesSinceLastSuccess( projectId, toId );
+
+        assertEquals( "no intervening failures, should return no changes", 0, changes.size() );
+    }
+
+    @Test
+    public void testGetChangesSinceLastSuccessInterveningFailures()
+        throws Exception
+    {
+        DefaultContinuum continuum = getContinuum();
+        continuum.setBuildResultDao( buildResultDao );
+
+        int projectId = 123, fromId = 789, toId = 1011;
+        BuildResult priorResult = new BuildResult();
+        priorResult.setId( fromId );
+
+        BuildResult[] failures = { resultWithChanges( 1 ), resultWithChanges( 0 ), resultWithChanges( 0, 1 ),
+            resultWithChanges( 1, 0 ), resultWithChanges( 0, 1, 0 ), resultWithChanges( 1, 0, 1 ) };
+
+        when( buildResultDao.getPreviousBuildResultInSuccess( projectId, toId ) ).thenReturn( priorResult );
+        when( buildResultDao.getBuildResultsForProjectWithDetails( projectId, fromId, toId ) ).thenReturn(
+            Arrays.asList( failures ) );
+
+        List<ChangeSet> changes = continuum.getChangesSinceLastSuccess( projectId, toId );
+
+        assertEquals( "should return same number of changes as in failed results", 6, changes.size() );
+        assertOldestToNewest( changes );
+    }
+
+    private static int changeCounter = 1;
+
+    private BuildResult resultWithChanges( int... changeSetCounts )
+    {
+        BuildResult result = new BuildResult();
+        ScmResult scmResult = new ScmResult();
+        result.setScmResult( scmResult );
+        for ( Integer changeCount : changeSetCounts )
+        {
+            for ( int i = 0; i < changeCount; i++ )
+            {
+                ChangeSet change = new ChangeSet();
+                change.setId( String.format( "%011d", changeCounter++ ) );  // zero-padded for string comparison
+                scmResult.addChange( change );
+            }
+        }
+        return result;
+    }
+
+    private void assertOldestToNewest( List<ChangeSet> changes )
+    {
+        if ( changes == null || changes.isEmpty() || changes.size() == 1 )
+            return;
+        for ( int prior = 0, next = 1; next < changes.size(); prior++, next = prior + 1 )
+        {
+            String priorId = changes.get( prior ).getId(), nextId = changes.get( next ).getId();
+            assertTrue( "changes were not in ascending order", priorId.compareTo( nextId ) < 0 );
+        }
+    }
+
+    private DefaultContinuum getContinuum()
         throws Exception
     {
-        return lookup( Continuum.class );
+        return (DefaultContinuum) lookup( Continuum.class );
     }
 
     private BuildResultDao getBuildResultDao()

Modified: continuum/trunk/continuum-store/src/main/java/org/apache/continuum/dao/BuildResultDaoImpl.java
URL: http://svn.apache.org/viewvc/continuum/trunk/continuum-store/src/main/java/org/apache/continuum/dao/BuildResultDaoImpl.java?rev=1677905&r1=1677904&r2=1677905&view=diff
==============================================================================
--- continuum/trunk/continuum-store/src/main/java/org/apache/continuum/dao/BuildResultDaoImpl.java (original)
+++ continuum/trunk/continuum-store/src/main/java/org/apache/continuum/dao/BuildResultDaoImpl.java Tue May  5 22:44:38 2015
@@ -689,7 +689,6 @@ public class BuildResultDaoImpl
     }
 
     public BuildResult getPreviousBuildResultInSuccess( int projectId, int buildResultId )
-        throws ContinuumStoreException
     {
         PersistenceManager pm = getPersistenceManager();
 

Modified: continuum/trunk/continuum-webapp/src/main/resources/localization/Continuum.properties
URL: http://svn.apache.org/viewvc/continuum/trunk/continuum-webapp/src/main/resources/localization/Continuum.properties?rev=1677905&r1=1677904&r2=1677905&view=diff
==============================================================================
--- continuum/trunk/continuum-webapp/src/main/resources/localization/Continuum.properties (original)
+++ continuum/trunk/continuum-webapp/src/main/resources/localization/Continuum.properties Tue May  5 22:44:38 2015
@@ -608,7 +608,7 @@ buildResult.dependencies.noChanges = No
 buildResult.dependencies.groupId = Group Id
 buildResult.dependencies.artifactId = Artifact Id
 buildResult.dependencies.version = Version
-buildResult.changesSinceLastSuccess = Other Changes Since Last Success
+buildResult.changesSinceLastSuccess = SCM Changes Since Last Success
 buildResult.generatedReports.title = Generated Reports
 buildResult.generatedReports.surefire = Surefire Report
 buildResult.buildOutput.text = Download as Text