-
-
Notifications
You must be signed in to change notification settings - Fork 509
Add logging to extractor #1403
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
base: dev
Are you sure you want to change the base?
Add logging to extractor #1403
Conversation
Set ConsoleLogger globally for all tests
79a16cb to
5ec7581
Compare
Add string formatting to logger Add tests for logger formatting
5ec7581 to
25b04f2
Compare
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.
LGTM, we should however add some documentation
- to the ExtractorLogger class and the template syntax
- on how to set the logger
- somehwere on how to disable the JUnit logger extension
| @@ -0,0 +1,193 @@ | |||
| package org.schabi.newpipe.extractor.utils; | |||
|
|
|||
| public final class ExtractorLogger { | |||
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.
A few lines of class doc would be good to have here.
TobiGr
left a comment
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.
Looks good to me, just a few nitpicks. This is good to merge once these and those above are applied.
|
|
||
| public static StreamInfo getInfo(@Nonnull final StreamExtractor extractor) | ||
| throws ExtractionException, IOException { | ||
| ExtractorLogger.d(TAG, "getInfo({extractor)", extractor); |
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.
| ExtractorLogger.d(TAG, "getInfo({extractor)", extractor); | |
| ExtractorLogger.d(TAG, "getInfo({extractor})", extractor); |
| @Test | ||
| void unmatchedBraceLeavesRemainder() { | ||
| ExtractorLogger.d("T", "Value {Unclosed", "X"); | ||
| assertEquals("Value {Unclosed", logger.lastDebug); | ||
| } |
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.
Please also add tests for templates which are not correctly formatted due to an incorrect number of opening or closing braces in addition to this test:
{{test}
{no_word}}
Adds a logging API to the Extractor.
By default it logs nothing, and users of the extractor can use setLogger to define where logs are written.
Uses ConsoleLogger for all tests by default using JUnit 5 extensions
Added some initial logs to some places.
Related PR: #1325