# HG changeset patch # User Severin Gehwolf # Date 1387536157 -3600 # Node ID 869c5e163e7ab8a16cc80beb33e6e46933b1dac9 # Parent b4c7ee6fb5c50d90a159b1a9d52098097e093d91 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 diff -r b4c7ee6fb5c5 -r 869c5e163e7a system-backend/src/main/java/com/redhat/thermostat/backend/system/DistributionInformation.java --- 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); } diff -r b4c7ee6fb5c5 -r 869c5e163e7a system-backend/src/main/java/com/redhat/thermostat/backend/system/EtcOsRelease.java --- 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 { diff -r b4c7ee6fb5c5 -r 869c5e163e7a system-backend/src/main/java/com/redhat/thermostat/backend/system/LsbRelease.java --- 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); diff -r b4c7ee6fb5c5 -r 869c5e163e7a system-backend/src/test/java/com/redhat/thermostat/backend/system/DistributionInformationTest.java --- 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() { diff -r b4c7ee6fb5c5 -r 869c5e163e7a system-backend/src/test/java/com/redhat/thermostat/backend/system/EtcOsReleaseTest.java --- 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)")); + } + } } diff -r b4c7ee6fb5c5 -r 869c5e163e7a system-backend/src/test/java/com/redhat/thermostat/backend/system/LsbReleaseTest.java --- 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")); + } + } } diff -r b4c7ee6fb5c5 -r 869c5e163e7a system-backend/src/test/java/com/redhat/thermostat/backend/system/TestLogHandler.java --- /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 + * . + * + * 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