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

Added MessageTemplateParameter IsValidTemplateTest unit test #3286

Closed
wants to merge 1 commit into from

Conversation

304NotModified
Copy link
Member

@304NotModified 304NotModified commented Apr 6, 2019

see #3181

todo:

@snakefoot

got some interesting results here:

image

With an empty list as object[] parameters , is always says IsValidTemplate=true - independent of the message-template. Is this expected? It this for performance reasons?


This change is Reviewable

@304NotModified 304NotModified changed the title Added tests IsPositionalTest + IsValidTemplateTest Added MessageTemplateParametersTests unit tests Apr 6, 2019
@snakefoot
Copy link
Contributor

snakefoot commented Apr 7, 2019

@304NotModified With an empty list as object[] parameters , is always says IsValidTemplate=true - independent of the message-template. Is this expected? It this for performance reasons?

Have no special opinion about MessageTemplateParameters it was created for the Seq-target that wanted access to the internal parser engine of NLog (To perform special transformation for parameters prefixed with @)

By default NLog will ignore message-template when no parameters are provided. This is used by many that just writes raw Json without parameters. Whether there should be any correlation between the behavior of MessageTemplateParameters and the default parser/renderer behavior of NLog that I don't know.

Your own special fix with GetHoleValueSafe should probably also handle isValidTemplate :)

Any changes you make to MessageTemplateParameters should just be validated against the behavior of the Seq-Target.

@304NotModified 304NotModified force-pushed the MessageTemplateParametersTests2 branch from 60b7546 to 235e2c6 Compare April 7, 2019 21:10
@304NotModified 304NotModified changed the title Added MessageTemplateParametersTests unit tests Added MessageTemplateParameter IsValidTemplateTest unit test Apr 7, 2019
@304NotModified
Copy link
Member Author

related: #2339

@304NotModified
Copy link
Member Author

closing for now, tracked here: #3375

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants