-
Notifications
You must be signed in to change notification settings - Fork 675
[Bug] Rule Toml Write Formatting Wrongly Formats \\\\x #4978
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
Changes from 6 commits
c6a2bdc
c19c8e3
2ced0dc
7bd4787
1965007
958fcfe
5dab229
1f0dca0
d69acdd
e6d22d9
f1501ca
b68278c
84da63c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,4 +71,12 @@ def test_formatter_rule(self): | |
|
|
||
| def test_formatter_deep(self): | ||
| """Test that the data remains unchanged from formatting.""" | ||
| self.compare_test_data(self.test_data[1:]) | ||
| self.compare_test_data(self.test_data[2:]) | ||
|
|
||
| def test_formatter_paths(self): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add another test to make sure
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this test not accomplish that? This output checks the need for If the The regex filter to catch the specific transformationThe Is this accomplishing what you are looking for? |
||
| """Test that paths are handled as expected in with toml lib.""" | ||
| with self.assertRaisesRegex( | ||
| AssertionError, | ||
| r'\+ {"metadata": {"field": "value"}, "rule": {"path": "\?:\\\\Windows\\\\Sys\?\?\?\?\?\\\\x5lrs\.dll"}}', | ||
| ): | ||
| self.compare_test_data([self.test_data[1]]) | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we will want a new unit test in
test_toml_formatter.pyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ agreed, added new unit test that looks for this specifically. It also will fail if the path behavior changes. E.g. currently we expect
\\\\to be formatted to\\. If this changes the unit test will fail on purpose as we want to match Query DSL's path handling which does this as well.