ZEST-190 Rewrite metrics library tests to fix their flakiness
Project: http://git-wip-us.apache.org/repos/asf/zest-java/repo Commit: http://git-wip-us.apache.org/repos/asf/zest-java/commit/812f1446 Tree: http://git-wip-us.apache.org/repos/asf/zest-java/tree/812f1446 Diff: http://git-wip-us.apache.org/repos/asf/zest-java/diff/812f1446 Branch: refs/heads/develop Commit: 812f1446cdffbe1a453a43fe9710772b7d9125d0 Parents: aff4282 Author: Paul Merlin <[email protected]> Authored: Mon Nov 7 13:51:42 2016 +0100 Committer: Paul Merlin <[email protected]> Committed: Mon Nov 7 13:58:41 2016 +0100 ---------------------------------------------------------------------- .../zest/test/metrics/MetricValuesProvider.java | 45 +++++ .../metrics/yammer/YammerMetricsAssembler.java | 16 +- .../metrics/AbstractTimingCaptureTest.java | 146 ++++++++++++++ .../library/metrics/DocumentationSupport.java | 2 +- .../zest/library/metrics/MetricsTest.java | 202 ------------------- .../metrics/YammerTimingCaptureTest.java | 76 +++++++ 6 files changed, 278 insertions(+), 209 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zest-java/blob/812f1446/core/testsupport/src/main/java/org/apache/zest/test/metrics/MetricValuesProvider.java ---------------------------------------------------------------------- diff --git a/core/testsupport/src/main/java/org/apache/zest/test/metrics/MetricValuesProvider.java b/core/testsupport/src/main/java/org/apache/zest/test/metrics/MetricValuesProvider.java new file mode 100644 index 0000000..25e15d0 --- /dev/null +++ b/core/testsupport/src/main/java/org/apache/zest/test/metrics/MetricValuesProvider.java @@ -0,0 +1,45 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * + */package org.apache.zest.test.metrics; + +import java.util.Collection; + +/** + * Metrics values provider. + */ +public interface MetricValuesProvider +{ + /** + * All registered metrics names. + * + * @return All registered metrics names + */ + Collection<String> registeredMetricNames(); + + /** + * Timer count. + * <p> + * Should not throw. + * Must return {@literal 0} if the timer does not exist. + * + * @param name Timer name + * @return Timer count + */ + long timerCount( String name ); +} http://git-wip-us.apache.org/repos/asf/zest-java/blob/812f1446/extensions/metrics-yammer/src/main/java/org/apache/zest/metrics/yammer/YammerMetricsAssembler.java ---------------------------------------------------------------------- diff --git a/extensions/metrics-yammer/src/main/java/org/apache/zest/metrics/yammer/YammerMetricsAssembler.java b/extensions/metrics-yammer/src/main/java/org/apache/zest/metrics/yammer/YammerMetricsAssembler.java index 76ebd86..5b32b5d 100644 --- a/extensions/metrics-yammer/src/main/java/org/apache/zest/metrics/yammer/YammerMetricsAssembler.java +++ b/extensions/metrics-yammer/src/main/java/org/apache/zest/metrics/yammer/YammerMetricsAssembler.java @@ -27,13 +27,13 @@ import com.yammer.metrics.reporting.CsvReporter; import java.io.File; import java.io.PrintStream; import java.util.concurrent.TimeUnit; -import org.apache.zest.api.common.Visibility; -import org.apache.zest.bootstrap.Assembler; +import org.apache.zest.bootstrap.Assemblers; import org.apache.zest.bootstrap.AssemblyException; import org.apache.zest.bootstrap.ModuleAssembly; +import org.apache.zest.bootstrap.ServiceDeclaration; public class YammerMetricsAssembler - implements Assembler + extends Assemblers.VisibilityIdentity<YammerMetricsAssembler> { private AbstractPollingReporter reporter; @@ -83,9 +83,13 @@ public class YammerMetricsAssembler public void assemble( ModuleAssembly module ) throws AssemblyException { - module.services( YammerMetricsProvider.class ) - .instantiateOnStartup() - .visibleIn( Visibility.application ); + ServiceDeclaration service = module.services(YammerMetricsProvider.class) + .instantiateOnStartup() + .visibleIn(visibility()); + if( hasIdentity() ) + { + service.identifiedBy( identity() ); + } } /** http://git-wip-us.apache.org/repos/asf/zest-java/blob/812f1446/libraries/metrics/src/test/java/org/apache/zest/library/metrics/AbstractTimingCaptureTest.java ---------------------------------------------------------------------- diff --git a/libraries/metrics/src/test/java/org/apache/zest/library/metrics/AbstractTimingCaptureTest.java b/libraries/metrics/src/test/java/org/apache/zest/library/metrics/AbstractTimingCaptureTest.java new file mode 100644 index 0000000..1b24419 --- /dev/null +++ b/libraries/metrics/src/test/java/org/apache/zest/library/metrics/AbstractTimingCaptureTest.java @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * + */ +package org.apache.zest.library.metrics; + +import org.apache.zest.api.common.Optional; +import org.apache.zest.api.composite.TransientComposite; +import org.apache.zest.api.mixin.Mixins; +import org.apache.zest.api.property.Property; +import org.apache.zest.bootstrap.Assembler; +import org.apache.zest.bootstrap.Assemblers; +import org.apache.zest.bootstrap.AssemblyException; +import org.apache.zest.bootstrap.ModuleAssembly; +import org.apache.zest.test.AbstractZestTest; +import org.apache.zest.test.metrics.MetricValuesProvider; +import org.junit.Test; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +abstract class AbstractTimingCaptureTest extends AbstractZestTest { + + @Override + public void assemble( ModuleAssembly module ) + throws AssemblyException + { + module.layer().application().setName( "SomeApplication" ); + module.transients( Country1.class ); + module.transients( Country2.class ).withConcerns( TimingCaptureAllConcern.class ); + module.transients( Country3.class ).withConcerns( TimingCaptureConcern.class ); + metricsAssembler().assemble( module ); + } + + protected abstract Assemblers.Visible<? extends Assembler> metricsAssembler(); + + protected abstract MetricValuesProvider metricValuesProvider(); + + @Test + public void givenNonInstrumentedCompositeExpectNoTimers() + { + Country underTest = transientBuilderFactory.newTransient( Country1.class ); + updateName( underTest, 10 ); + assertThat( metricValuesProvider().timerCount( "Layer 1.Module 1.AbstractTimingCaptureTest.Country.name" ), is( 0L ) ); + assertThat( metricValuesProvider().timerCount( "Layer 1.Module 1.AbstractTimingCaptureTest.Country.updateName" ), is( 0L ) ); + } + + @Test + public void givenInstrumentedWithAllCompositeWhenCallingUpdateNameExpectTimers() + { + Country underTest = transientBuilderFactory.newTransient( Country2.class ); + updateName( underTest, 10 ); + assertThat( metricValuesProvider().timerCount( "Layer 1.Module 1.AbstractTimingCaptureTest.Country.name" ), is( 10L ) ); + assertThat( metricValuesProvider().timerCount( "Layer 1.Module 1.AbstractTimingCaptureTest.Country.updateName" ), is( 10L ) ); + } + + @Test + public void givenOneMethodAnnotatedWhenCallingUpdateNameExpectTimerForThatMethodOnly() + { + Country underTest = transientBuilderFactory.newTransient( Country3.class ); + updateName( underTest, 10 ); + assertThat( metricValuesProvider().timerCount( "Layer 1.Module 1.AbstractTimingCaptureTest.Country.name" ), is( 0L ) ); + assertThat( metricValuesProvider().timerCount( "Country3.updateName" ), is( 10L ) ); + } + + private void updateName( Country underTest, int times ) + { + for( int i = 0; i < times; i++ ) + { + underTest.updateName( "Name" + i ); + } + } + + // START SNIPPET: complex-capture + public interface Country extends TransientComposite + { + @Optional + Property<String> name(); + + void updateName( String newName ); + } + + @Mixins( Country1Mixin.class ) + public interface Country1 extends Country + { + } + + public static abstract class Country1Mixin + implements Country1 + { + @Override + public void updateName( String newName ) + { + name().set( newName ); + } + } + + @Mixins( Country2Mixin.class ) + public interface Country2 extends Country + { + } + + public static abstract class Country2Mixin + implements Country2 + { + @Override + public void updateName( String newName ) + { + name().set( newName ); + } + } + + @Mixins( Country3Mixin.class ) + public interface Country3 extends Country + { + @TimingCapture( "Country3.updateName" ) + @Override + void updateName(String newName); + } + + public static abstract class Country3Mixin + implements Country3 + { + @Override + public void updateName( String newName ) + { + name().set( newName ); + } + } + // END SNIPPET: complex-capture +} http://git-wip-us.apache.org/repos/asf/zest-java/blob/812f1446/libraries/metrics/src/test/java/org/apache/zest/library/metrics/DocumentationSupport.java ---------------------------------------------------------------------- diff --git a/libraries/metrics/src/test/java/org/apache/zest/library/metrics/DocumentationSupport.java b/libraries/metrics/src/test/java/org/apache/zest/library/metrics/DocumentationSupport.java index dd99baf..48db537 100644 --- a/libraries/metrics/src/test/java/org/apache/zest/library/metrics/DocumentationSupport.java +++ b/libraries/metrics/src/test/java/org/apache/zest/library/metrics/DocumentationSupport.java @@ -58,7 +58,7 @@ public class DocumentationSupport } // END SNIPPET: capture - public class MyAsembler implements Assembler + public class MyAssembler implements Assembler { // START SNIPPET: capture @Override http://git-wip-us.apache.org/repos/asf/zest-java/blob/812f1446/libraries/metrics/src/test/java/org/apache/zest/library/metrics/MetricsTest.java ---------------------------------------------------------------------- diff --git a/libraries/metrics/src/test/java/org/apache/zest/library/metrics/MetricsTest.java b/libraries/metrics/src/test/java/org/apache/zest/library/metrics/MetricsTest.java deleted file mode 100644 index 1aa22bd..0000000 --- a/libraries/metrics/src/test/java/org/apache/zest/library/metrics/MetricsTest.java +++ /dev/null @@ -1,202 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * - */ - -package org.apache.zest.library.metrics; - -import com.yammer.metrics.Metrics; -import com.yammer.metrics.core.MetricsRegistry; -import java.io.ByteArrayOutputStream; -import java.io.PrintStream; -import java.lang.reflect.Field; -import java.util.Map; -import java.util.concurrent.TimeUnit; -import org.junit.ClassRule; -import org.junit.Test; -import org.apache.zest.api.common.Optional; -import org.apache.zest.api.composite.TransientComposite; -import org.apache.zest.api.concern.Concerns; -import org.apache.zest.api.mixin.Mixins; -import org.apache.zest.api.property.Property; -import org.apache.zest.bootstrap.AssemblyException; -import org.apache.zest.bootstrap.ModuleAssembly; -import org.apache.zest.metrics.yammer.YammerMetricsAssembler; -import org.apache.zest.test.AbstractZestTest; -import org.apache.zest.test.util.RetryRule; - -import static org.hamcrest.CoreMatchers.equalTo; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - -public class MetricsTest extends AbstractZestTest -{ - @ClassRule public static RetryRule retry = new RetryRule(); - - private PrintStream reportOut; - private ByteArrayOutputStream result; - private YammerMetricsAssembler assembler; - - @Override - public void assemble( ModuleAssembly module ) - throws AssemblyException - { - module.layer().application().setName( "SomeApplication" ); - module.transients( Country1.class ); - module.transients( Country2.class ).withConcerns( TimingCaptureAllConcern.class ); - module.transients( Country3.class ).withConcerns( TimingCaptureConcern.class ); - result = new ByteArrayOutputStream(); - reportOut = new PrintStream( result ); - assembler = new YammerMetricsAssembler( reportOut, 100, TimeUnit.MILLISECONDS ); - assembler.assemble( module ); - } - - @Override - public void tearDown() - throws Exception - { - if( assembler != null ) // Is null if assemble fails... - { - assembler.shutdown(); - } - Field metrics = MetricsRegistry.class.getDeclaredField( "metrics" ); - metrics.setAccessible( true ); - Map m = (Map) metrics.get( Metrics.defaultRegistry() ); - m.clear(); - super.tearDown(); - } - - @Test - public void givenNonInstrumentedCompositeWhenCallingUpdateNameExpectNoReport() - { - Country underTest = transientBuilderFactory.newTransient( Country1.class ); - String result = runTest( underTest ); - result = result.replace( "\r", "" ); - System.out.println( result ); - assertTrue( lastLine( result, 1 ).contains( "=====================" ) ); - System.out.println( "---END TEST---" ); - } - - @Test - public void givenInstrumentedWithAllCompositeWhenCallingUpdateNameExpectReport() - { - Country underTest = transientBuilderFactory.newTransient( Country2.class ); - String result = runTest( underTest ); - result = result.replace( "\r", "" ); - System.out.println( result ); - assertThat( lastLine( result, 33 ).trim(), equalTo( "Layer 1.Module 1.MetricsTest.Country.name:" ) ); - assertThat( lastLine( result, 16 ).trim(), equalTo( "Layer 1.Module 1.MetricsTest.Country.updateName:" ) ); - assertTrue( lastLine( result, 5 ).contains( "75% <=" ) ); - assertTrue( lastLine( result, 4 ).contains( "95% <=" ) ); - assertTrue( lastLine( result, 3 ).contains( "98% <=" ) ); - assertTrue( lastLine( result, 2 ).contains( "99% <=" ) ); - assertTrue( lastLine( result, 1 ).contains( "99.9% <=" ) ); - System.out.println( "---END TEST---" ); - } - - @Test - public void givenOneMethodAnnotatedWhenCallingUpdateNameExpectReportForThatMethodOnly() - { - Country underTest = transientBuilderFactory.newTransient( Country3.class ); - String result = runTest( underTest ); - result = result.replace( "\r", "" ); - System.out.println( result ); - assertThat( lastLine( result, 16 ).trim(), equalTo( "Country3.updateName:" ) ); - assertTrue( lastLine( result, 5 ).contains( "75% <=" ) ); - assertTrue( lastLine( result, 4 ).contains( "95% <=" ) ); - assertTrue( lastLine( result, 3 ).contains( "98% <=" ) ); - assertTrue( lastLine( result, 2 ).contains( "99% <=" ) ); - assertTrue( lastLine( result, 1 ).contains( "99.9% <=" ) ); - System.out.println( "---END TEST---" ); - } - - private String lastLine( String text, int index ) - { - String[] lines = text.split( "\n" ); - return lines[ lines.length - index ]; - } - - private String runTest( Country underTest ) - { - for( int i = 0; i < 1000000; i++ ) - { - underTest.updateName( "Name" + i ); - } - reportOut.close(); - return result.toString(); - } - -// START SNIPPET: complex-capture - public interface Country extends TransientComposite - { - @Optional - Property<String> name(); - - void updateName( String newName ); - } - - @Mixins( Country1Mixin.class ) - public interface Country1 extends Country - { - } - - public static abstract class Country1Mixin - implements Country1 - { - @Override - public void updateName( String newName ) - { - name().set( newName ); - } - } - - @Mixins( Country2Mixin.class ) - public interface Country2 extends Country - { - } - - public static abstract class Country2Mixin - implements Country2 - { - @Override - public void updateName( String newName ) - { - name().set( newName ); - } - } - - @Mixins( Country3Mixin.class ) - @Concerns( TimingCaptureConcern.class ) - public interface Country3 extends Country - { - @TimingCapture( "Country3.updateName" ) - @Override - void updateName(String newName); - } - - public static abstract class Country3Mixin - implements Country3 - { - @Override - public void updateName( String newName ) - { - name().set( newName ); - } - } -// END SNIPPET: complex-capture -} http://git-wip-us.apache.org/repos/asf/zest-java/blob/812f1446/libraries/metrics/src/test/java/org/apache/zest/library/metrics/YammerTimingCaptureTest.java ---------------------------------------------------------------------- diff --git a/libraries/metrics/src/test/java/org/apache/zest/library/metrics/YammerTimingCaptureTest.java b/libraries/metrics/src/test/java/org/apache/zest/library/metrics/YammerTimingCaptureTest.java new file mode 100644 index 0000000..7f1b711 --- /dev/null +++ b/libraries/metrics/src/test/java/org/apache/zest/library/metrics/YammerTimingCaptureTest.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * + */ +package org.apache.zest.library.metrics; + +import com.yammer.metrics.Metrics; +import com.yammer.metrics.core.MetricName; +import com.yammer.metrics.core.MetricsRegistry; +import org.apache.zest.bootstrap.Assembler; +import org.apache.zest.bootstrap.Assemblers; +import org.apache.zest.metrics.yammer.YammerMetricsAssembler; +import org.apache.zest.test.metrics.MetricValuesProvider; + +import java.lang.reflect.Field; +import java.util.Collection; +import java.util.Map; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.stream.Collectors.toList; + +public class YammerTimingCaptureTest extends AbstractTimingCaptureTest +{ + @Override + protected Assemblers.Visible<? extends Assembler> metricsAssembler() + { + return new YammerMetricsAssembler(); + } + + @Override + protected MetricValuesProvider metricValuesProvider() + { + MetricsRegistry metricsRegistry = Metrics.defaultRegistry(); + return new MetricValuesProvider() + { + @Override + public Collection<String> registeredMetricNames() + { + return metricsRegistry.allMetrics().keySet().stream().map( MetricName::getName ).collect( toList() ); + } + + @Override + public long timerCount( String timerName ) + { + MetricName metricName = new MetricName( "", "", timerName ); + return metricsRegistry.newTimer( metricName, MILLISECONDS, MILLISECONDS ).count(); + } + }; + } + + @Override + public void tearDown() + throws Exception + { + Field metrics = MetricsRegistry.class.getDeclaredField( "metrics" ); + metrics.setAccessible( true ); + Map m = (Map) metrics.get( Metrics.defaultRegistry() ); + m.clear(); + super.tearDown(); + } +}
