Inline error service action #827
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #827 +/- ##
==========================================
- Coverage 81.39% 81.00% -0.40%
==========================================
Files 427 426 -1
Lines 11391 11414 +23
Branches 1691 1690 -1
==========================================
- Hits 9272 9246 -26
- Misses 1411 1458 +47
- Partials 708 710 +2
|
| @@ -60,10 +63,7 @@ internal class LogRecordExporterTest { | |||
|
|
|||
| sleep(3000) | |||
| } | |||
| Assert.assertTrue((fakeLogRecordExporter.exportedLogs?.size ?: 0) == 0) | |||
| Assert.assertTrue(fake.msgQueue.any { | |||
There was a problem hiding this comment.
If there were previous test logic that was inspecting what internal errors were logged, how do we do this now?
There was a problem hiding this comment.
This can be achieved with changes in #828 as it replaces the logger with an interface. IMO the check isn't super helpful here as it's just asserting that a message was logged. I would like to make it clearer that we are logging internal errors & make it easier to test, but that come in additional refactoring
There was a problem hiding this comment.
Got it. There should be other side effects that can be observed, eh? I suppose if logging is the only thing that isn't verified, it's not that bad.
fractalwrench
force-pushed
the
inline-err-service-action
branch
2 times, most recently
from
7eefd31
to
f44b97e
Compare
fractalwrench
force-pushed
the
inline-err-service-action
branch
from
70c4dca
to
f74d08b
Compare
Goal
Inlines the call to the
InternalErrorServiceintoInternalEmbraceLogger, which simplifies the code & avoids a call to an iterator each time the log function is called.Testing
Updated existing test coverage.