You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by rf...@apache.org on 2019/12/18 22:15:59 UTC

[maven] 05/12: Log fail-level option. Submitted by: Martin Kanters.

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 <mk...@gmail.com>
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