changeset 1381:869c5e163e7a

Don't log WARNING if /etc/os-release missing, but fallback used. Reviewed-by: omajid Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-December/009059.html PR1629
author Severin Gehwolf <sgehwolf@redhat.com>
date Fri, 20 Dec 2013 11:42:37 +0100
parents b4c7ee6fb5c5
children 7caa7f9afb14
files system-backend/src/main/java/com/redhat/thermostat/backend/system/DistributionInformation.java system-backend/src/main/java/com/redhat/thermostat/backend/system/EtcOsRelease.java system-backend/src/main/java/com/redhat/thermostat/backend/system/LsbRelease.java system-backend/src/test/java/com/redhat/thermostat/backend/system/DistributionInformationTest.java system-backend/src/test/java/com/redhat/thermostat/backend/system/EtcOsReleaseTest.java system-backend/src/test/java/com/redhat/thermostat/backend/system/LsbReleaseTest.java system-backend/src/test/java/com/redhat/thermostat/backend/system/TestLogHandler.java
diffstat 7 files changed, 248 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/system-backend/src/main/java/com/redhat/thermostat/backend/system/DistributionInformation.java	Tue Dec 03 15:25:02 2013 +0100
+++ b/system-backend/src/main/java/com/redhat/thermostat/backend/system/DistributionInformation.java	Fri Dec 20 11:42:37 2013 +0100
@@ -58,15 +58,25 @@
     }
 
     public static DistributionInformation get() {
+        EtcOsRelease etcOsRelease = new EtcOsRelease();
+        LsbRelease lsbRelease = new LsbRelease();
+        return get(etcOsRelease, lsbRelease);
+    }
+    
+    // package-private for testing
+    static DistributionInformation get(EtcOsRelease etcOsRelease, LsbRelease lsbRelease) {
         try {
-            return new EtcOsRelease().getDistributionInformation();
+            return etcOsRelease.getDistributionInformation();
         } catch (IOException e) {
-            logger.log(Level.WARNING, "unable to use os-release", e);
+            // Log only at level FINE, since we have the LSB fallback
+            logger.log(Level.FINE, "unable to use os-release", e);
         }
         try {
-            return new LsbRelease().getDistributionInformation();
+            return lsbRelease.getDistributionInformation();
         } catch (IOException e) {
-            logger.log(Level.WARNING, "unable to use lsb_release", e);
+            // Log exception at level FINE only.
+            logger.log(Level.FINE, "unable to use lsb_release", e);
+            logger.log(Level.WARNING, "unable to use os-release AND lsb_release");
         }
         return new DistributionInformation(UNKNOWN_NAME, UNKNOWN_VERSION);
     }
--- a/system-backend/src/main/java/com/redhat/thermostat/backend/system/EtcOsRelease.java	Tue Dec 03 15:25:02 2013 +0100
+++ b/system-backend/src/main/java/com/redhat/thermostat/backend/system/EtcOsRelease.java	Fri Dec 20 11:42:37 2013 +0100
@@ -45,14 +45,24 @@
 public class EtcOsRelease implements DistributionInformationSource {
 
     private static final String OS_RELEASE = "/etc/os-release";
-
+    private final String osReleaseFile;
+    
+    public EtcOsRelease() {
+        this.osReleaseFile = OS_RELEASE;
+    }
+    
+    // package-private for testing
+    EtcOsRelease(String osReleaseFile) {
+        this.osReleaseFile = osReleaseFile;
+    }
+    
     @Override
     public DistributionInformation getDistributionInformation() throws IOException {
         return getFromOsRelease();
     }
 
     public DistributionInformation getFromOsRelease() throws IOException {
-        return getFromOsRelease(OS_RELEASE);
+        return getFromOsRelease(osReleaseFile);
     }
 
     public DistributionInformation getFromOsRelease(String releaseFile) throws IOException {
--- a/system-backend/src/main/java/com/redhat/thermostat/backend/system/LsbRelease.java	Tue Dec 03 15:25:02 2013 +0100
+++ b/system-backend/src/main/java/com/redhat/thermostat/backend/system/LsbRelease.java	Fri Dec 20 11:42:37 2013 +0100
@@ -51,6 +51,18 @@
 
     private static final String DISTRIBUTION_NAME = "distributor id";
     private static final String DISTRIBUTION_VERSION = "release";
+    private static final String LSB_RELEASE_SCRIPT = "lsb_release";
+    
+    private final String lsbRelaseBin;
+    
+    public LsbRelease() {
+        this.lsbRelaseBin = LSB_RELEASE_SCRIPT;
+    }
+    
+    // package-private for testing
+    LsbRelease(String lsbReleaseBin) {
+        this.lsbRelaseBin = lsbReleaseBin;
+    }
 
     @Override
     public DistributionInformation getDistributionInformation()
@@ -59,10 +71,9 @@
     }
 
     public DistributionInformation getFromLsbRelease() throws IOException {
-
         BufferedReader reader = null;
         try {
-            Process lsbProc = Runtime.getRuntime().exec(new String[] { "lsb_release", "-a" });
+            Process lsbProc = Runtime.getRuntime().exec(new String[] { lsbRelaseBin, "-a" });
             InputStream progOutput = lsbProc.getInputStream();
             reader = new BufferedReader(new InputStreamReader(progOutput));
             DistributionInformation result = getFromLsbRelease(reader);
--- a/system-backend/src/test/java/com/redhat/thermostat/backend/system/DistributionInformationTest.java	Tue Dec 03 15:25:02 2013 +0100
+++ b/system-backend/src/test/java/com/redhat/thermostat/backend/system/DistributionInformationTest.java	Fri Dec 20 11:42:37 2013 +0100
@@ -38,11 +38,94 @@
 
 import static org.junit.Assert.*;
 
+import java.util.logging.Handler;
+import java.util.logging.Logger;
+
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 
 import com.redhat.thermostat.testutils.TestUtils;
 
 public class DistributionInformationTest {
+    
+    private Logger logger;
+    private TestLogHandler handler;
+    
+    @Before
+    public void setup() {
+        setupTestLogger();
+    }
+    
+    @After
+    public void tearDown() {
+        if (handler != null) {
+            logger.removeHandler(handler);
+            handler = null;
+        }
+    }
+    
+    private void setupTestLogger() {
+        logger = Logger.getLogger("com.redhat.thermostat");
+        handler = new TestLogHandler();
+        logger.addHandler(handler);
+    }
+    
+    /*
+     * Verifies that no warning gets logged if EtcOsRelease fails, but
+     * LsbRelease works. Since LsbRelease is the fallback, all is well.
+     * 
+     * see: http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1628
+     */
+    @Test
+    public void testNoWarningLoggedOnFallback() {
+        // verify preconditions
+        assertFalse(handler.isEtcOsReleaseLogged());
+        assertTestHandlerRegistered();
+        
+        // Default LSB release
+        LsbRelease lsbRelease = new LsbRelease();
+        // EtcOsRelease with non existent file
+        EtcOsRelease etcOsRelease = new EtcOsRelease(EtcOsReleaseTest.NOT_EXISTING_OS_RELEASE_FILE);
+        DistributionInformation.get(etcOsRelease, lsbRelease);
+        assertFalse(handler.isEtcOsReleaseLogged());
+    }
+    
+    /*
+     * Verifies that a warning gets logged if os-release and lsb_release both
+     * fail.
+     * 
+     * see: http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1628
+     */
+    @Test
+    public void testWarningLoggedIfBothFail() {
+        // verify preconditions
+        assertFalse(handler.isEtcOsReleaseLogged());
+        assertFalse(handler.isLsbReleaseLogged());
+        assertTestHandlerRegistered();
+        
+        // both etc-os-release and lsb-release don't exist for this test
+        EtcOsRelease etcOsRelease = new EtcOsRelease(EtcOsReleaseTest.NOT_EXISTING_OS_RELEASE_FILE);
+        LsbRelease lsbRelease = new LsbRelease(LsbReleaseTest.NOT_EXISTING_LSB_RELEASE);
+        
+        DistributionInformation info = DistributionInformation.get(etcOsRelease, lsbRelease);
+        assertFalse(handler.isEtcOsReleaseLogged());
+        assertTrue(handler.isLsbReleaseLogged());
+        assertNotNull(info);
+        assertEquals(DistributionInformation.UNKNOWN_NAME, info.getName());
+        assertEquals(DistributionInformation.UNKNOWN_VERSION, info.getVersion());
+    }
+
+    private void assertTestHandlerRegistered() {
+        assertNotNull(logger);
+        boolean testLogHandlerRegistered = false;
+        for (Handler h: logger.getHandlers()) {
+            if (h instanceof TestLogHandler) {
+                testLogHandlerRegistered = true;
+            }
+        }
+        assertTrue(testLogHandlerRegistered);
+    }
 
     @Test
     public void testName() {
--- a/system-backend/src/test/java/com/redhat/thermostat/backend/system/EtcOsReleaseTest.java	Tue Dec 03 15:25:02 2013 +0100
+++ b/system-backend/src/test/java/com/redhat/thermostat/backend/system/EtcOsReleaseTest.java	Fri Dec 20 11:42:37 2013 +0100
@@ -37,16 +37,22 @@
 package com.redhat.thermostat.backend.system;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.StringReader;
+import java.util.UUID;
 
 import org.junit.Test;
 
 import com.redhat.thermostat.test.Bug;
 
 public class EtcOsReleaseTest {
+    
+    static final String NOT_EXISTING_OS_RELEASE_FILE = "/thermostat-os-release-testing-"
+            + UUID.randomUUID();
 
     @Test
     public void testName() throws IOException, InterruptedException {
@@ -80,6 +86,20 @@
         assertEquals("openSUSE", info.getName());
         assertEquals("12.1 (Asparagus)", info.getVersion());
     }
+    
+    @Test
+    public void getDistributionInformationThrowsIOExceptionIfFileNotThere() {
+        EtcOsRelease etcOsRelease = new EtcOsRelease(NOT_EXISTING_OS_RELEASE_FILE);
+        try {
+            etcOsRelease.getDistributionInformation();
+            fail("Should have thrown IOException, since file is not there!");
+        } catch (IOException e) {
+            // pass
+            String message = e.getMessage();
+            assertTrue(message.contains("/thermostat-os-release-testing-"));
+            assertTrue(message.contains("(No such file or directory)"));
+        }
+    }
 
 }
 
--- a/system-backend/src/test/java/com/redhat/thermostat/backend/system/LsbReleaseTest.java	Tue Dec 03 15:25:02 2013 +0100
+++ b/system-backend/src/test/java/com/redhat/thermostat/backend/system/LsbReleaseTest.java	Fri Dec 20 11:42:37 2013 +0100
@@ -37,14 +37,20 @@
 package com.redhat.thermostat.backend.system;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.StringReader;
+import java.util.UUID;
 
 import org.junit.Test;
 
 public class LsbReleaseTest {
+    
+    static final String NOT_EXISTING_LSB_RELEASE = "lsb_release-"
+            + UUID.randomUUID();
 
     @Test
     public void testName() throws IOException, InterruptedException {
@@ -59,6 +65,20 @@
         DistributionInformation info = new LsbRelease().getFromLsbRelease(reader);
         assertEquals("Version", info.getVersion());
     }
+    
+    @Test
+    public void getDistributionInformationThrowsIOExceptionIfScriptNotThere() {
+        LsbRelease lsbRelease = new LsbRelease(NOT_EXISTING_LSB_RELEASE);
+        try {
+            lsbRelease.getDistributionInformation();
+            fail("Should have thrown IOException, since file is not there!");
+        } catch (IOException e) {
+            // pass
+            String message = e.getMessage();
+            assertTrue(message.contains("Cannot run program \"lsb_release-"));
+            assertTrue(message.contains("No such file or directory"));
+        }
+    }
 
 }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/system-backend/src/test/java/com/redhat/thermostat/backend/system/TestLogHandler.java	Fri Dec 20 11:42:37 2013 +0100
@@ -0,0 +1,86 @@
+/*
+ * Copyright 2012, 2013 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.backend.system;
+
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+
+/*
+ * Test log handler used for DistributionInformation log testing.
+ */
+public class TestLogHandler extends Handler {
+    
+    private boolean etcOsReleaseLogged;
+    private boolean lsbReleaseLogged;
+    private static final String EXPECTED_OS_RELEASE_FAIL_MSG =
+            "unable to use os-release";
+    private static final String EXPECTED_LSB_RELEASE_FAIL_MSG =
+            "unable to use os-release AND lsb_release";
+    
+    @Override
+    public void publish(LogRecord record) {
+        String logMessage = record.getMessage();
+        if (record.getLevel().intValue() >= Level.WARNING.intValue() && 
+                logMessage.equals(EXPECTED_OS_RELEASE_FAIL_MSG)) {
+            etcOsReleaseLogged = true;
+        };
+        if (record.getLevel().intValue() >= Level.WARNING.intValue() &&
+                logMessage.equals(EXPECTED_LSB_RELEASE_FAIL_MSG)) {
+            lsbReleaseLogged = true;
+        }
+    }
+
+    @Override
+    public void flush() {
+        // nothing
+    }
+
+    @Override
+    public void close() throws SecurityException {
+        // nothing
+    }
+    
+    boolean isEtcOsReleaseLogged() {
+        return etcOsReleaseLogged;
+    }
+    
+    boolean isLsbReleaseLogged() {
+        return lsbReleaseLogged;
+    }
+    
+}
\ No newline at end of file