From 0db9c91b9a999a88a1557db6f70330d129b2f9a4 Mon Sep 17 00:00:00 2001 From: simonbrandhof Date: Sat, 4 Dec 2010 11:26:20 +0000 Subject: [PATCH] SONAR-249 add some unit tests --- .../core/timemachine/PastMeasuresLoader.java | 4 ++ .../core/timemachine/VariationDecorator.java | 35 ++++++----- .../timemachine/VariationDecoratorTest.java | 59 +++++++++++++++++++ .../sonar/api/database/model/Snapshot.java | 38 ++++++++---- .../java/org/sonar/api/measures/Metric.java | 36 ++++++++--- 5 files changed, 136 insertions(+), 36 deletions(-) diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/PastMeasuresLoader.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/PastMeasuresLoader.java index be7fcfe30b1..7fc073956fc 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/PastMeasuresLoader.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/PastMeasuresLoader.java @@ -56,6 +56,10 @@ public class PastMeasuresLoader implements BatchExtension { return metricByIds.values(); } + public List getPastMeasures(Resource resource, PastSnapshot projectPastSnapshot) { + return getPastMeasures(resource, projectPastSnapshot.getProjectSnapshot()); + } + public List getPastMeasures(Resource resource, Snapshot projectSnapshot) { // assume that the resource has already been saved return getPastMeasures(resource.getId(), projectSnapshot); diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/VariationDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/VariationDecorator.java index 03b594c36bf..12635f88ff5 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/VariationDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/VariationDecorator.java @@ -39,16 +39,16 @@ import java.util.Map; @DependedUpon(DecoratorBarriers.END_OF_TIME_MACHINE) public class VariationDecorator implements Decorator { - private List targets; + private List projectPastSnapshots; private PastMeasuresLoader pastMeasuresLoader; public VariationDecorator(PastMeasuresLoader pastMeasuresLoader, TimeMachineConfiguration configuration) { this(pastMeasuresLoader, configuration.getVariationSnapshots()); } - VariationDecorator(PastMeasuresLoader pastMeasuresLoader, List targets) { + VariationDecorator(PastMeasuresLoader pastMeasuresLoader, List projectPastSnapshots) { this.pastMeasuresLoader = pastMeasuresLoader; - this.targets = targets; + this.projectPastSnapshots = projectPastSnapshots; } public boolean shouldExecuteOnProject(Project project) { @@ -60,22 +60,22 @@ public class VariationDecorator implements Decorator { return pastMeasuresLoader.getMetrics(); } - static boolean shouldCalculateVariations(Resource resource) { - // measures on files are currently purged, so past measures are not available on files - return !ResourceUtils.isEntity(resource); - } - public void decorate(Resource resource, DecoratorContext context) { if (shouldCalculateVariations(resource)) { - for (PastSnapshot target : targets) { - calculateVariation(resource, context, target); + for (PastSnapshot projectPastSnapshot : projectPastSnapshots) { + calculateVariation(resource, context, projectPastSnapshot); } } } - private void calculateVariation(Resource resource, DecoratorContext context, PastSnapshot target) { - List pastMeasures = pastMeasuresLoader.getPastMeasures(resource, target.getProjectSnapshot()); - compareWithPastMeasures(context, target.getIndex(), pastMeasures); + static boolean shouldCalculateVariations(Resource resource) { + // measures on files are currently purged, so past measures are not available on files + return !ResourceUtils.isEntity(resource); + } + + private void calculateVariation(Resource resource, DecoratorContext context, PastSnapshot pastSnapshot) { + List pastMeasures = pastMeasuresLoader.getPastMeasures(resource, pastSnapshot); + compareWithPastMeasures(context, pastSnapshot.getIndex(), pastMeasures); } void compareWithPastMeasures(DecoratorContext context, int index, List pastMeasures) { @@ -88,16 +88,19 @@ public class VariationDecorator implements Decorator { for (Measure measure : context.getMeasures(MeasuresFilters.all())) { // compare with past measure MeasureModel pastMeasure = pastMeasuresByKey.get(new MeasureKey(measure)); - updateVariation(measure, pastMeasure, index); - context.saveMeasure(measure); + if (updateVariation(measure, pastMeasure, index)) { + context.saveMeasure(measure); + } } } - void updateVariation(Measure measure, MeasureModel pastMeasure, int index) { + boolean updateVariation(Measure measure, MeasureModel pastMeasure, int index) { if (pastMeasure != null && pastMeasure.getValue() != null && measure.getValue() != null) { double variation = (measure.getValue().doubleValue() - pastMeasure.getValue().doubleValue()); measure.setVariation(index, variation); + return true; } + return false; } @Override diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/VariationDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/VariationDecoratorTest.java index b4f93aa97db..a2d21bcb804 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/VariationDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/VariationDecoratorTest.java @@ -20,14 +20,28 @@ package org.sonar.plugins.core.timemachine; import org.junit.Test; +import org.mockito.Matchers; +import org.sonar.api.batch.DecoratorContext; +import org.sonar.api.database.model.MeasureModel; +import org.sonar.api.database.model.Snapshot; +import org.sonar.api.measures.Measure; +import org.sonar.api.measures.MeasuresFilter; +import org.sonar.api.measures.Metric; import org.sonar.api.resources.*; import org.sonar.jpa.test.AbstractDbUnitTestCase; +import java.util.Arrays; + import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsNull.nullValue; +import static org.mockito.Mockito.*; public class VariationDecoratorTest extends AbstractDbUnitTestCase { + public static final Metric NCLOC = new Metric("ncloc").setId(12); + public static final Metric COVERAGE = new Metric("coverage").setId(16); + @Test public void shouldNotCalculateVariationsOnFiles() { assertThat(VariationDecorator.shouldCalculateVariations(new Project("foo")), is(true)); @@ -39,4 +53,49 @@ public class VariationDecoratorTest extends AbstractDbUnitTestCase { assertThat(VariationDecorator.shouldCalculateVariations(new File("org/foo/Bar.php")), is(false)); } + @Test + public void shouldCompareAndSaveVariation() { + Resource javaPackage = new JavaPackage("org.foo"); + + PastMeasuresLoader pastMeasuresLoader = mock(PastMeasuresLoader.class); + PastSnapshot pastSnapshot1 = new PastSnapshot(1, "days", new Snapshot()); + PastSnapshot pastSnapshot3 = new PastSnapshot(3, "days", new Snapshot()); + + // first past analysis + when(pastMeasuresLoader.getPastMeasures(javaPackage, pastSnapshot1)).thenReturn(Arrays.asList( + newMeasureModel(NCLOC, 180.0), + newMeasureModel(COVERAGE, 75.0))); + + // second past analysis + when(pastMeasuresLoader.getPastMeasures(javaPackage, pastSnapshot3)).thenReturn(Arrays.asList( + newMeasureModel(NCLOC, 240.0))); + + // current analysis + DecoratorContext context = mock(DecoratorContext.class); + Measure currentNcloc = newMeasure(NCLOC, 200.0); + Measure currentCoverage = newMeasure(COVERAGE, 80.0); + when(context.getMeasures(Matchers.anyObject())).thenReturn(Arrays.asList(currentNcloc, currentCoverage)); + + VariationDecorator decorator = new VariationDecorator(pastMeasuresLoader, Arrays.asList(pastSnapshot1, pastSnapshot3)); + decorator.decorate(javaPackage, context); + + // context updated for each variation : 2 times for ncloc and 1 time for coverage + verify(context, times(3)).saveMeasure(Matchers.anyObject()); + + assertThat(currentNcloc.getVariation1(), is(20.0)); + assertThat(currentNcloc.getVariation2(), nullValue()); + assertThat(currentNcloc.getVariation3(), is(-40.0)); + + assertThat(currentCoverage.getVariation1(), is(5.0)); + assertThat(currentCoverage.getVariation2(), nullValue()); + assertThat(currentCoverage.getVariation3(), nullValue()); + } + + private Measure newMeasure(Metric metric, double value) { + return new Measure(metric, value); + } + + private MeasureModel newMeasureModel(Metric metric, double value) { + return new MeasureModel(metric.getId(), value); + } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/database/model/Snapshot.java b/sonar-plugin-api/src/main/java/org/sonar/api/database/model/Snapshot.java index ce45562be13..9700f23d424 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/database/model/Snapshot.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/database/model/Snapshot.java @@ -25,8 +25,8 @@ import org.apache.commons.lang.builder.ToStringBuilder; import org.sonar.api.database.BaseIdentifiable; import org.sonar.api.database.DatabaseSession; -import java.util.Date; import javax.persistence.*; +import java.util.Date; /** * A class to map a snapshot with its hibernate model @@ -160,86 +160,97 @@ public class Snapshot extends BaseIdentifiable { return createdAt; } - public void setCreatedAt(Date createdAt) { + public Snapshot setCreatedAt(Date createdAt) { this.createdAt = createdAt; + return this; } public Integer getResourceId() { return resourceId; } - public void setResourceId(Integer resourceId) { + public Snapshot setResourceId(Integer resourceId) { this.resourceId = resourceId; + return this; } - public final void setResource(ResourceModel resource) { + public final Snapshot setResource(ResourceModel resource) { this.resourceId = resource.getId(); this.scope = resource.getScope(); this.qualifier = resource.getQualifier(); + return this; } public String getVersion() { return version; } - public void setVersion(String version) { + public Snapshot setVersion(String version) { this.version = version; + return this; } public Integer getParentId() { return parentId; } - public void setParentId(Integer i) { + public Snapshot setParentId(Integer i) { this.parentId = i; + return this; } public Boolean getLast() { return last; } - public void setLast(Boolean last) { + public Snapshot setLast(Boolean last) { this.last = last; + return this; } public String getStatus() { return status; } - public void setStatus(String status) { + public Snapshot setStatus(String status) { this.status = status; + return this; } public String getScope() { return scope; } - public void setScope(String scope) { + public Snapshot setScope(String scope) { this.scope = scope; + return this; } public String getQualifier() { return qualifier; } - public void setQualifier(String qualifier) { + public Snapshot setQualifier(String qualifier) { this.qualifier = qualifier; + return this; } public Integer getRootId() { return rootId; } - public void setRootId(Integer i) { + public Snapshot setRootId(Integer i) { this.rootId = i; + return this; } public String getPath() { return path; } - public void setPath(String path) { + public Snapshot setPath(String path) { this.path = path; + return this; } public Integer getDepth() { @@ -250,8 +261,9 @@ public class Snapshot extends BaseIdentifiable { return rootProjectId; } - public void setRootProjectId(Integer rootProjectId) { + public Snapshot setRootProjectId(Integer rootProjectId) { this.rootProjectId = rootProjectId; + return this; } /** diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/measures/Metric.java b/sonar-plugin-api/src/main/java/org/sonar/api/measures/Metric.java index 87ff5bd61dc..73305be154a 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/measures/Metric.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/measures/Metric.java @@ -22,7 +22,6 @@ package org.sonar.api.measures; import org.apache.commons.lang.builder.ToStringBuilder; import org.sonar.api.BatchExtension; import org.sonar.api.ServerExtension; -import org.sonar.api.database.BaseIdentifiable; import javax.persistence.*; @@ -31,7 +30,7 @@ import javax.persistence.*; */ @Table(name = "metrics") @Entity(name = "Metric") -public class Metric extends BaseIdentifiable implements ServerExtension, BatchExtension { +public class Metric implements ServerExtension, BatchExtension { /** * A metric bigger value means a degradation @@ -68,6 +67,11 @@ public class Metric extends BaseIdentifiable implements ServerExtension, BatchEx JAV, GUI, WS } + @Id + @Column(name = "id") + @GeneratedValue + private Integer id; + @Transient private Formula formula; @@ -119,7 +123,8 @@ public class Metric extends BaseIdentifiable implements ServerExtension, BatchEx /** * Creates an empty metric */ - @Deprecated public Metric() { + @Deprecated + public Metric() { } /** @@ -173,8 +178,8 @@ public class Metric extends BaseIdentifiable implements ServerExtension, BatchEx this.userManaged = userManaged; this.origin = Origin.JAV; if (ValueType.PERCENT.equals(this.type)) { - this.bestValue = (direction==DIRECTION_BETTER ? 100.0 : 0.0); - this.worstValue = (direction==DIRECTION_BETTER ? 0.0 : 100.0); + this.bestValue = (direction == DIRECTION_BETTER ? 100.0 : 0.0); + this.worstValue = (direction == DIRECTION_BETTER ? 0.0 : 100.0); } } @@ -206,11 +211,27 @@ public class Metric extends BaseIdentifiable implements ServerExtension, BatchEx this.userManaged = false; this.formula = formula; if (ValueType.PERCENT.equals(this.type)) { - this.bestValue = (direction==DIRECTION_BETTER ? 100.0 : 0.0); - this.worstValue = (direction==DIRECTION_BETTER ? 0.0 : 100.0); + this.bestValue = (direction == DIRECTION_BETTER ? 100.0 : 0.0); + this.worstValue = (direction == DIRECTION_BETTER ? 0.0 : 100.0); } } + /** + * For internal use only + */ + public Integer getId() { + return id; + } + + /** + * For internal use only + */ + public Metric setId(Integer id) { + this.id = id; + return this; + } + + /** * @return the metric formula */ @@ -513,6 +534,7 @@ public class Metric extends BaseIdentifiable implements ServerExtension, BatchEx /** * Merge with fields from other metric. All fields are copied, except the id. + * * @return this */ public Metric merge(final Metric with) { -- 2.39.5