Introduce common code for Recorders in Plugin Tests #3347
Conversation
fendor
requested review from
kokobd,
Ailrun,
VeryMilkyJoe and
drsooch
as code owners
hls-test-utils/src/Test/Hls.hs
Outdated
| -- The created recorder writes to stderr if any of the given environment variables | ||
| -- have been set to a value different to @0@. |
There was a problem hiding this comment.
Why not write to stderr unconditionally, as the default recorder for HLS does?
There was a problem hiding this comment.
Lots of noise, makes it hard to tell what testcase failed, imo. However, thats also fine by me
There was a problem hiding this comment.
But this is already the status quo, and lsp-test silences it anyway, so I'm not sure what the difference is
There was a problem hiding this comment.
No it is not, the status quo is, we hide the stderr unless LSP_TEST_LOG_STDERR=1 is given, take a look at this example run with the loggers for HLS and the testsuite writing to stderr vs not writing to stderr: https://paste.tomsmeding.com/0UbJEowE
Argh, I forgot we hadn't purged all of that yet >:( |
|
I think "always have a recorder" is a good default tbh, so 👍 for refactoring things to make that the main way. |
fendor
force-pushed
the
refactor/test-plugin-loggers
branch
from
aa69bda to
24a81b2
Compare
fendor
requested review from
ozkutuk,
uhbif19,
isovector,
konn,
santiweight,
OliverMadine,
eddiemundo,
berberman,
georgefst and
July541
as code owners
fendor
force-pushed
the
refactor/test-plugin-loggers
branch
2 times, most recently
from
d4d5ce1 to
fe57732
Compare
|
Ready for review, each commit modifies a single plugin so code owners can review exactly the commit they need to. For reviewers, the most important changes happen in hls-test-utils/src/Test/Hls.hs |
fendor
force-pushed
the
refactor/test-plugin-loggers
branch
from
fe57732 to
108baf6
Compare
Every plugin should use the new functions. This way, we can guarantee predictable logging behaviour for tests. Most notably, we now have a single control point, for co-log style logging, how to write logs to stderr, and how to write logs for server and plugin at the same time.
fendor
force-pushed
the
refactor/test-plugin-loggers
branch
from
108baf6 to
93d2587
Compare
fendor
force-pushed
the
refactor/test-plugin-loggers
branch
from
93d2587 to
1c4c1d1
Compare
Plugin Tests need a recorder. Most plugin test-suite just set it to
mempty, which, I think, will make it more difficult to debug test-failures.Some plugin test-suites roll their own plugin initialisation code. This is fine, but makes the logging behaviour non-uniform.
Introduce the
PluginTestDescriptortype definition.Utility functions to create
PluginTestDescriptor'smkPluginTestDescriptorandmkPluginTestDescriptor'which will help migrating plugins to the newer co-log style logger.