-
Notifications
You must be signed in to change notification settings - Fork 11
feat: add logfmt parser #51
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: main
Are you sure you want to change the base?
Conversation
b9ebb96 to
c715ae9
Compare
e7ee688 to
59f95bc
Compare
|
It would be good to have a parametrized round-trip test - that is, take a (valid) dict of strings to strings, logfmt it, then parse_logfmt, and check that the dict you get back is the dict you put in. One could then construct variously unpleasant dictionaries and verify that they came through all right. One could also add a check that invalid dictionaries that didn't make sense in logfmt (empty string as key?) raise a sensible exception. |
|
My test already compares the format with the parsed result. |
59f95bc to
e701961
Compare
Indeed. But round-trip tests are a verification of both parts of the system, and are much easier to construct difficult examples for. |
|
I can easily split the tests case and make the round trip: but I currently already also verify the exact resulting string. |
e701961 to
cbebdb4
Compare
|
@td-anne I added two more additional round trip tests |
|
@spaceone once this is merged with latest (and conflicts resolved), it should pass the tests (since newline update has been merged) |
If we really wanted this, then we would probably have some normalization before comparing the strings, but I don't think its necessary (at least for initial rollout). |
cbebdb4 to
aa9b26c
Compare
|
@josheppinette I rebased it. But I don't think it's complete. We should speak about some behavior:
I can't easily detect this case.
|
>>> logfmter.Logfmter.format_params({'a': '\\', 'b': 'c'})
'a=\\ b=c'Similar to the newline issue, I would have expected: >>> logfmter.Logfmter.format_params({'a': '\\', 'b': 'c'})
'a="\\\\" b=c'Would you consider this change as valid? |
$ echo 'foo="\\" bar=baz' | ./golang-logfmt-echo
foo=\ bar=baz |
aa9b26c to
1f907a5
Compare
1f907a5 to
34f039c
Compare
Fixes: #28
Depends on #49 and #50