-
-
Notifications
You must be signed in to change notification settings - Fork 765
Add SLF4J logging support for Appium local service #1014
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
385e81e
97981ee
294bcf6
f93ed2e
9455c34
a27cd6f
954cc60
aec7b55
7268231
606322e
0739c0c
0ecaa89
e3ac85b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
package io.appium.java_client.service.local; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.slf4j.event.Level; | ||
|
||
/** | ||
* This class provides context to a Slf4jLogMessageConsumer. | ||
* | ||
*/ | ||
public final class Slf4jLogMessageContext { | ||
private final Logger logger; | ||
private final Level level; | ||
|
||
Slf4jLogMessageContext(String loggerName, Level level) { | ||
this.level = level; | ||
this.logger = LoggerFactory.getLogger(loggerName); | ||
} | ||
|
||
/** | ||
* Returns the {@link Logger} instance associated with this context. | ||
* | ||
* @return {@link Logger} instance associated with this context. | ||
* | ||
*/ | ||
public Logger getLogger() { | ||
return logger; | ||
} | ||
|
||
/** | ||
* Returns log {@link Level} for the log message associated with this context. | ||
* | ||
* @return {@link Level} for log message associated with this context. | ||
*/ | ||
public Level getLevel() { | ||
return level; | ||
} | ||
|
||
/** | ||
* Returns the name of the {@link Logger} associated with this context. | ||
* | ||
* @return name of {@link Logger} associated with this context. | ||
*/ | ||
public String getName() { | ||
return logger.getName(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package io.appium.java_client.service.local; | ||
|
||
import static io.appium.java_client.service.local.AppiumDriverLocalService.parseSlf4jContextFromLogMessage; | ||
import static org.junit.Assert.assertEquals; | ||
import static org.slf4j.LoggerFactory.getLogger; | ||
import static org.slf4j.event.Level.DEBUG; | ||
import static org.slf4j.event.Level.INFO; | ||
|
||
import org.junit.Test; | ||
import org.slf4j.event.Level; | ||
|
||
public class AppiumDriverLocalServiceTest { | ||
|
||
@Test | ||
public void canParseSlf4jLoggerContext() throws Exception { | ||
assertLoggerContext(INFO, "appium.service.androidbootstrap", | ||
"[AndroidBootstrap] [BOOTSTRAP LOG] [debug] json loading complete."); | ||
assertLoggerContext(INFO, "appium.service.adb", | ||
"[ADB] Cannot read version codes of "); | ||
assertLoggerContext(INFO, "appium.service.xcuitest", | ||
"[XCUITest] Determining device to run tests on: udid: '1234567890', real device: true"); | ||
assertLoggerContext(INFO, "appium.service", | ||
"no-prefix log message."); | ||
assertLoggerContext(INFO, "appium.service", | ||
"no-prefix log [not-a-logger-name] message."); | ||
assertLoggerContext(DEBUG, "appium.service.mjsonwp", | ||
"[debug] [MJSONWP] Calling AppiumDriver.getStatus() with args: []"); | ||
assertLoggerContext(DEBUG, "appium.service.xcuitest", | ||
"[debug] [XCUITest] Xcode version set to 'x.y.z' "); | ||
assertLoggerContext(DEBUG, "appium.service.jsonwpproxy", | ||
"[debug] [JSONWP Proxy] Proxying [GET /status] to [GET http://localhost:18218/status] with no body"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would you parse messages with timestamps like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as mentioned in javadoc. this feature is not supported when from javadoc: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
|
||
private void assertLoggerContext(Level expectedLevel, String expectedLoggerName, String logMessage) { | ||
Slf4jLogMessageContext ctx = parseSlf4jContextFromLogMessage(logMessage); | ||
assertEquals(expectedLoggerName, ctx.getName()); | ||
assertEquals(expectedLevel, ctx.getLevel()); | ||
assertEquals(getLogger(expectedLoggerName), ctx.getLogger()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,7 @@ | |
import java.net.InetAddress; | ||
import java.net.NetworkInterface; | ||
import java.nio.file.Path; | ||
import java.util.ArrayList; | ||
import java.util.Enumeration; | ||
import java.util.List; | ||
|
||
|
@@ -114,6 +115,15 @@ public void tearDown() throws Exception { | |
ofNullable(PATH_TO_APPIUM_NODE_IN_PROPERTIES).ifPresent(s -> setProperty(APPIUM_PATH, s)); | ||
} | ||
|
||
@Test public void checkAbilityToAddLogMessageConsumer() { | ||
List<String> log = new ArrayList<>(); | ||
service = buildDefaultService(); | ||
service.clearOutPutStreams(); | ||
service.addLogMessageConsumer(log::add); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we also have a possibility to remove a consumer? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Essentially a consumer in this case is an OutputStream (as added with |
||
service.start(); | ||
assertTrue(log.size() > 0); | ||
} | ||
|
||
@Test public void checkAbilityToStartDefaultService() { | ||
service = buildDefaultService(); | ||
service.start(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is anyway not very accurate, since this
[debug]
prefix is also not always added to debug logsUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mykola-mokhnach So what do you sugests instead? Should all be logged at
INFO
level? As it is now log messages with[debug]
prefix is logged atDEBUG
level. That is better than nothing if you ask me.