-
Notifications
You must be signed in to change notification settings - Fork 57
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
Fix: ride hail transit re-enable #3790
Conversation
fix for #3781 |
test! |
8 similar comments
test! |
test! |
test! |
test! |
test! |
test! |
test! |
test! |
test! |
1 similar comment
test! |
test! |
test! |
1 similar comment
test! |
test! |
test! |
test! |
test! |
test! |
test! |
test! |
test! |
1 similar comment
test! |
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.
If what I suggested seems too complicated feel free to skip it.
def logInfo(message: String): Unit = logger.info(message) | ||
|
||
// required for testing purposes | ||
def logError(message: String): Unit = logger.error(message) |
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.
Basically we can use it. Though it looks a little strange. What we can do is to create a custom Logback appender (https://www.baeldung.com/custom-logback-appender) and in logback-test.xml add it to the PopulationAdjustment only. Then we can get the appender programmatically and cleanup/check it before/after running a test.
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.
Previously it did not work with custom appender and logger in the test (the approach was different though), and it looks like suggested change is too much work just to get one test working
This change is