This is an automated email from the ASF dual-hosted git repository. rfscholte pushed a commit to branch MNG-6065 in repository https://gitbox.apache.org/repos/asf/maven.git
commit d043df945a92be5a2c34540ef8bf6b832b891a16 Author: Martin Kanters <[email protected]> AuthorDate: Sun Dec 1 16:15:58 2019 +0100 Log fail-level option. Submitted by: Martin Kanters. Moved the fail level thresholds logic into the LogLevelRecorder class instead of the factory. --- .../org/slf4j/impl/MavenLoggerFactoryTest.java | 38 +++++++-------------- .../main/java/org/apache/maven/cli/MavenCli.java | 8 +++-- .../maven/cli/event/ExecutionEventLogger.java | 15 +++++---- .../java/org/slf4j/impl/MavenFailLevelLogger.java | 1 + .../java/org/slf4j/impl/MavenLoggerFactory.java | 25 +++++--------- .../apache/maven/logwrapper}/LogLevelRecorder.java | 19 +++++++---- .../maven/logwrapper/MavenSlf4jWrapperFactory.java | 7 ++-- .../maven/logwrapper/LogLevelRecorderTest.java | 39 +++++++++++----------- 8 files changed, 70 insertions(+), 82 deletions(-) diff --git a/apache-maven/src/test/java/org/slf4j/impl/MavenLoggerFactoryTest.java b/apache-maven/src/test/java/org/slf4j/impl/MavenLoggerFactoryTest.java index 8baf624..67ce825 100644 --- a/apache-maven/src/test/java/org/slf4j/impl/MavenLoggerFactoryTest.java +++ b/apache-maven/src/test/java/org/slf4j/impl/MavenLoggerFactoryTest.java @@ -19,6 +19,7 @@ package org.slf4j.impl; * under the License. */ +import org.apache.maven.logwrapper.LogLevelRecorder; import org.junit.Test; import org.slf4j.Logger; @@ -58,47 +59,32 @@ public class MavenLoggerFactoryTest } @Test - public void createsFailLevelLogger() - { - MavenLoggerFactory mavenLoggerFactory = new MavenLoggerFactory(); - mavenLoggerFactory.breakOnLogLevel( "WARN" ); - - Logger logger = mavenLoggerFactory.getLogger( "Test" ); - - assertThat( logger, instanceOf( MavenFailLevelLogger.class ) ); - } - - @Test public void reportsWhenFailLevelHasBeenHit() { MavenLoggerFactory mavenLoggerFactory = new MavenLoggerFactory(); - mavenLoggerFactory.breakOnLogLevel( "ERROR" ); + mavenLoggerFactory.setLogLevelRecorder( new LogLevelRecorder( "ERROR" ) ); + + assertTrue( mavenLoggerFactory.getLogLevelRecorder().isPresent() ); + LogLevelRecorder logLevelRecorder = mavenLoggerFactory.getLogLevelRecorder().get(); MavenFailLevelLogger logger = ( MavenFailLevelLogger ) mavenLoggerFactory.getLogger( "Test" ); - assertFalse( mavenLoggerFactory.isThresholdHit() ); + assertFalse( logLevelRecorder.isThresholdHit() ); logger.warn( "This should not hit the fail level" ); - assertFalse( mavenLoggerFactory.isThresholdHit() ); + assertFalse( logLevelRecorder.isThresholdHit() ); logger.error( "This should hit the fail level" ); - assertTrue( mavenLoggerFactory.isThresholdHit() ); + assertTrue( logLevelRecorder.isThresholdHit() ); logger.warn( "This should not reset the fail level" ); - assertTrue( mavenLoggerFactory.isThresholdHit() ); + assertTrue( logLevelRecorder.isThresholdHit() ); } @Test( expected = IllegalStateException.class ) public void failLevelThresholdCanOnlyBeSetOnce() { MavenLoggerFactory mavenLoggerFactory = new MavenLoggerFactory(); - mavenLoggerFactory.breakOnLogLevel( "WARN" ); - mavenLoggerFactory.breakOnLogLevel( "ERROR" ); - } - - @Test( expected = IllegalArgumentException.class ) - public void onlyWarningOrHigherFailLevelsCanBeSet() - { - MavenLoggerFactory mavenLoggerFactory = new MavenLoggerFactory(); - mavenLoggerFactory.breakOnLogLevel( "INFO" ); + mavenLoggerFactory.setLogLevelRecorder( new LogLevelRecorder( "WARN" ) ); + mavenLoggerFactory.setLogLevelRecorder( new LogLevelRecorder( "ERROR" ) ); } -} \ No newline at end of file +} diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java index c18d74f..34f5561 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java @@ -58,6 +58,7 @@ import org.apache.maven.execution.scope.internal.MojoExecutionScopeModule; import org.apache.maven.extension.internal.CoreExports; import org.apache.maven.extension.internal.CoreExtensionEntry; import org.apache.maven.lifecycle.LifecycleExecutionException; +import org.apache.maven.logwrapper.LogLevelRecorder; import org.apache.maven.logwrapper.MavenSlf4jWrapperFactory; import org.apache.maven.model.building.ModelProcessor; import org.apache.maven.project.MavenProject; @@ -546,12 +547,13 @@ public class MavenCli if ( cliRequest.commandLine.hasOption( CLIManager.FAIL_LEVEL ) ) { - String logLevelToBreakOn = cliRequest.commandLine.getOptionValue( CLIManager.FAIL_LEVEL ); + String logLevelThreshold = cliRequest.commandLine.getOptionValue( CLIManager.FAIL_LEVEL ); if ( slf4jLoggerFactory instanceof MavenSlf4jWrapperFactory ) { - ( (MavenSlf4jWrapperFactory) slf4jLoggerFactory ).breakOnLogLevel( logLevelToBreakOn ); - slf4jLogger.info( "Enabled to break the build on log level {}.", logLevelToBreakOn ); + LogLevelRecorder logLevelRecorder = new LogLevelRecorder( logLevelThreshold ); + ( (MavenSlf4jWrapperFactory) slf4jLoggerFactory ).setLogLevelRecorder( logLevelRecorder ); + slf4jLogger.info( "Enabled to break the build on log level {}.", logLevelThreshold ); } } } diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java b/maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java index 2422fba..dc41021 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/event/ExecutionEventLogger.java @@ -33,6 +33,7 @@ import org.apache.maven.execution.BuildSummary; import org.apache.maven.execution.ExecutionEvent; import org.apache.maven.execution.MavenExecutionResult; import org.apache.maven.execution.MavenSession; +import org.apache.maven.logwrapper.LogLevelRecorder; import org.apache.maven.logwrapper.MavenSlf4jWrapperFactory; import org.apache.maven.plugin.MojoExecution; import org.apache.maven.plugin.descriptor.MojoDescriptor; @@ -138,12 +139,14 @@ public class ExecutionEventLogger extends AbstractExecutionListener if ( iLoggerFactory instanceof MavenSlf4jWrapperFactory ) { - if ( ( (MavenSlf4jWrapperFactory) iLoggerFactory ).isThresholdHit() ) - { - event.getSession().getResult().addException( new Exception( - "Build failed due to log statements with a higher severity than allowed. " - + "Fix the logged issues or remove flag --fail-level (-fl)." ) ); - } + MavenSlf4jWrapperFactory loggerFactory = (MavenSlf4jWrapperFactory) iLoggerFactory; + loggerFactory.getLogLevelRecorder() + .filter( LogLevelRecorder::isThresholdHit ) + .ifPresent(recorder -> + event.getSession().getResult().addException( new Exception( + "Build failed due to log statements with a higher severity than allowed. " + + "Fix the logged issues or remove flag --fail-level (-fl)." ) ) + ); } logResult( event.getSession() ); diff --git a/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenFailLevelLogger.java b/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenFailLevelLogger.java index 7cfa608..b4b4605 100644 --- a/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenFailLevelLogger.java +++ b/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenFailLevelLogger.java @@ -19,6 +19,7 @@ package org.slf4j.impl; * under the License. */ +import org.apache.maven.logwrapper.LogLevelRecorder; import org.slf4j.event.Level; /** diff --git a/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenLoggerFactory.java b/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenLoggerFactory.java index cf3760a..914f2a5 100644 --- a/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenLoggerFactory.java +++ b/maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenLoggerFactory.java @@ -19,9 +19,11 @@ package org.slf4j.impl; * under the License. */ +import org.apache.maven.logwrapper.LogLevelRecorder; import org.apache.maven.logwrapper.MavenSlf4jWrapperFactory; import org.slf4j.Logger; -import org.slf4j.event.Level; + +import java.util.Optional; /** * LogFactory for Maven which can create a simple logger or a one which, if set, fails the build on a threshold. @@ -31,31 +33,20 @@ public class MavenLoggerFactory extends SimpleLoggerFactory implements MavenSlf4 private LogLevelRecorder logLevelRecorder = null; @Override - public void breakOnLogLevel( String logLevelToBreakOn ) + public void setLogLevelRecorder( LogLevelRecorder logLevelRecorder ) { - if ( logLevelRecorder != null ) + if ( this.logLevelRecorder != null ) { throw new IllegalStateException( "Maven logger fail level has already been set." ); } - Level level = Level.valueOf( logLevelToBreakOn ); - if ( level.toInt() < Level.WARN.toInt() ) - { - throw new IllegalArgumentException( "Logging level thresholds can only be set to WARN or ERROR" ); - } - - logLevelRecorder = new LogLevelRecorder( level ); + this.logLevelRecorder = logLevelRecorder; } @Override - public boolean isThresholdHit() + public Optional<LogLevelRecorder> getLogLevelRecorder() { - if ( logLevelRecorder != null ) - { - return logLevelRecorder.isThresholdHit(); - } - - return false; + return Optional.ofNullable( logLevelRecorder ); } /** diff --git a/maven-slf4j-provider/src/main/java/org/slf4j/impl/LogLevelRecorder.java b/maven-slf4j-wrapper/src/main/java/org/apache/maven/logwrapper/LogLevelRecorder.java similarity index 72% copy from maven-slf4j-provider/src/main/java/org/slf4j/impl/LogLevelRecorder.java copy to maven-slf4j-wrapper/src/main/java/org/apache/maven/logwrapper/LogLevelRecorder.java index 652fee7..0ac2957 100644 --- a/maven-slf4j-provider/src/main/java/org/slf4j/impl/LogLevelRecorder.java +++ b/maven-slf4j-wrapper/src/main/java/org/apache/maven/logwrapper/LogLevelRecorder.java @@ -1,4 +1,4 @@ -package org.slf4j.impl; +package org.apache.maven.logwrapper; /* * Licensed to the Apache Software Foundation (ASF) under one @@ -24,18 +24,23 @@ import org.slf4j.event.Level; /** * Responsible for keeping state of whether the threshold of the --fail-level flag has been hit. */ -class LogLevelRecorder +public class LogLevelRecorder { private final Level logThreshold; private boolean thresholdHit = false; - LogLevelRecorder( Level logLevel ) + public LogLevelRecorder( String threshold ) { - assert logLevel != null; - this.logThreshold = logLevel; + Level level = Level.valueOf( threshold ); + if ( level.toInt() < Level.WARN.toInt() ) + { + throw new IllegalArgumentException( "Logging level thresholds can only be set to WARN or ERROR" ); + } + + logThreshold = level; } - void record( Level logLevel ) + public void record( Level logLevel ) { if ( !thresholdHit && logLevel.toInt() >= logThreshold.toInt() ) { @@ -43,7 +48,7 @@ class LogLevelRecorder } } - boolean isThresholdHit() + public boolean isThresholdHit() { return thresholdHit; } diff --git a/maven-slf4j-wrapper/src/main/java/org/apache/maven/logwrapper/MavenSlf4jWrapperFactory.java b/maven-slf4j-wrapper/src/main/java/org/apache/maven/logwrapper/MavenSlf4jWrapperFactory.java index bdd1520..e2063b7 100644 --- a/maven-slf4j-wrapper/src/main/java/org/apache/maven/logwrapper/MavenSlf4jWrapperFactory.java +++ b/maven-slf4j-wrapper/src/main/java/org/apache/maven/logwrapper/MavenSlf4jWrapperFactory.java @@ -21,12 +21,13 @@ package org.apache.maven.logwrapper; import org.slf4j.ILoggerFactory; +import java.util.Optional; + /** * Wrapper for creating loggers which can have a log level threshold. */ public interface MavenSlf4jWrapperFactory extends ILoggerFactory { - boolean isThresholdHit(); - - void breakOnLogLevel( String logLevelToBreakOn ); + void setLogLevelRecorder( LogLevelRecorder logLevelRecorder ); + Optional<LogLevelRecorder> getLogLevelRecorder(); } diff --git a/maven-slf4j-provider/src/main/java/org/slf4j/impl/LogLevelRecorder.java b/maven-slf4j-wrapper/src/test/java/org/apache/maven/logwrapper/LogLevelRecorderTest.java similarity index 54% rename from maven-slf4j-provider/src/main/java/org/slf4j/impl/LogLevelRecorder.java rename to maven-slf4j-wrapper/src/test/java/org/apache/maven/logwrapper/LogLevelRecorderTest.java index 652fee7..db5337c 100644 --- a/maven-slf4j-provider/src/main/java/org/slf4j/impl/LogLevelRecorder.java +++ b/maven-slf4j-wrapper/src/test/java/org/apache/maven/logwrapper/LogLevelRecorderTest.java @@ -1,4 +1,4 @@ -package org.slf4j.impl; +package org.apache.maven.logwrapper; /* * Licensed to the Apache Software Foundation (ASF) under one @@ -9,7 +9,7 @@ package org.slf4j.impl; * "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 + * 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 @@ -19,32 +19,31 @@ package org.slf4j.impl; * under the License. */ +import org.junit.Test; import org.slf4j.event.Level; -/** - * Responsible for keeping state of whether the threshold of the --fail-level flag has been hit. - */ -class LogLevelRecorder -{ - private final Level logThreshold; - private boolean thresholdHit = false; +import static org.junit.Assert.assertTrue; - LogLevelRecorder( Level logLevel ) +public class LogLevelRecorderTest +{ + @Test + public void createsLogLevelRecorder() { - assert logLevel != null; - this.logThreshold = logLevel; + LogLevelRecorder logLevelRecorder = new LogLevelRecorder( "WARN" ); + logLevelRecorder.record( Level.ERROR ); + + assertTrue( logLevelRecorder.isThresholdHit() ); } - void record( Level logLevel ) + @Test( expected = IllegalArgumentException.class ) + public void failsOnLowerThanWarn () { - if ( !thresholdHit && logLevel.toInt() >= logThreshold.toInt() ) - { - thresholdHit = true; - } + new LogLevelRecorder( "INFO" ); } - boolean isThresholdHit() + @Test( expected = IllegalArgumentException.class ) + public void failsOnUnknownLogLevel () { - return thresholdHit; + new LogLevelRecorder( "SEVERE" ); } -} +} \ No newline at end of file
