Skip to content
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

Add logback config for eclair-node tests #1377

Merged
merged 1 commit into from
Apr 15, 2020
Merged

Add logback config for eclair-node tests #1377

merged 1 commit into from
Apr 15, 2020

Conversation

pm47
Copy link
Member

@pm47 pm47 commented Apr 15, 2020

Otherwise we use the default logback.xml file which writes to a
rolling file in the user directory.

NB: when enabling console logging, there are a lot of WARN outputs. Maybe something to look into @araspitzu?

10:37:33.657 WARN  ActorSystemImpl - withRequestTimeout was used in route however no request-timeout is set!
10:37:33.657 WARN  ActorSystemImpl - withRequestTimeoutResponse was used in route however no request-timeout is set!
10:37:33.683 WARN  ActorSystemImpl - withRequestTimeout was used in route however no request-timeout is set!
10:37:33.683 WARN  ActorSystemImpl - withRequestTimeoutResponse was used in route however no request-timeout is set!         

Otherwise we use the default logback.xml file which writes to a
rolling file in the user directory.
@pm47 pm47 requested a review from araspitzu April 15, 2020 08:44
Copy link
Contributor

@araspitzu araspitzu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that warning is suspicious, i'll look into it!

@codecov-io
Copy link

Codecov Report

Merging #1377 into master will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1377      +/-   ##
==========================================
+ Coverage   86.45%   86.51%   +0.05%     
==========================================
  Files         123      123              
  Lines        9357     9357              
  Branches      405      405              
==========================================
+ Hits         8090     8095       +5     
+ Misses       1267     1262       -5     
Impacted Files Coverage Δ
.../eclair/payment/relay/PostRestartHtlcCleaner.scala 85.43% <0.00%> (-1.33%) ⬇️
...clair/payment/send/MultiPartPaymentLifecycle.scala 97.70% <0.00%> (-0.58%) ⬇️
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 85.66% <0.00%> (+0.36%) ⬆️
...main/scala/fr/acinq/eclair/router/Validation.scala 92.51% <0.00%> (+1.57%) ⬆️

@araspitzu
Copy link
Contributor

The reason is that when we test the ServiceSpec we use route.TestKit instead of starting an actual HTTP server, this is the correct choice but there is a known issue with timeouts and akka-http testing akka/akka-http#952

@pm47
Copy link
Member Author

pm47 commented Apr 15, 2020

Fair enough, merging

@pm47 pm47 merged commit 9cb14ee into master Apr 15, 2020
@pm47 pm47 deleted the node-test-log branch April 15, 2020 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants