HelpersTask579-add-unit-tests-for-sort_config_string#645
HelpersTask579-add-unit-tests-for-sort_config_string#645
Conversation
| key2: | ||
| key3: | ||
| key4: | ||
|
|
There was a problem hiding this comment.
Did you remove these empty lines or was that Linter? Here and also twice below
There was a problem hiding this comment.
No I didn't, it was the linter.
There was a problem hiding this comment.
Pls let's revert these Linter changes. I'll file an issue because it's some erroneous formatting
|
|
||
| class TestSortConfigString(hunitest.TestCase): | ||
| """ | ||
| Test sort_config_string function with various config formats. |
There was a problem hiding this comment.
Pls use backticks for the function name
| """ | ||
| Helper method to test sorted config output. | ||
| """ | ||
| result = cconfig.sort_config_string(txt) |
There was a problem hiding this comment.
By convention, we call it actual
|
|
||
|
|
||
| # ############################################################################# | ||
| # TestSortConfigString |
There was a problem hiding this comment.
The file where this class is located is wrong. Since the original function is in config_utils.py, its test should be in test_config_utils.py
| # ############################################################################# | ||
|
|
||
|
|
||
| class TestSortConfigString(hunitest.TestCase): |
There was a problem hiding this comment.
Test cases themselves look good. Feel free to fix the bug in the function and you can use these tests to confirm that the fix works.
| wrong_indent: value2 | ||
| config2: | ||
| extra_indent: value3 | ||
| single: value4 | ||
| bad_indent: value5 |
There was a problem hiding this comment.
Is it expected that wrong_indent and bad_indent get removed? My understanding was that the function is only about sorting alphabetically. Does it also validate indentation? Maybe it can be added to the docstring in that case.
sonniki
added
PR for authors
2 participants
Link to Issue: #579
Added unit tests, the existing function may need to be revised.
FYI @sonniki @samarth9008 @gpsaggese