fix incorrect combination of lines in flux resource status for expandable fields that differ past the minimum width
#6638
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks! I've set MWP. |
|
This pull request has been removed from the queue for the following reason: The pull request #6638 has been manually updated You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. If you want to requeue this pull request, you need to post a comment with the text: |
Problem: The OutputFormat copy() method takes an except_fields list of fields to suppress in the copy, but this can leave prefix sentinels like `+:` and `?:` and extra whitespace in the result. Suppress extra text from skipped fields if it is all whitespace or whitespace plus a supperted sentinel prefix.
Problem: The expansion sentinels `+:` and `?+:` allow an output
format field to expand to the maximum width in a given list of
formatted items. However, when a single item is formatted, any minimum
width, as specified in the format spec, will cause the result to be
truncated. This stymies attempts to find identical lines when the
lines are not filtered first.
Add options to the get_format_prepended() and copy() methods to
"nullify" the expansion sentinel by replacing the field without any
width and precision specifiers, i.e. `+:{foo:<3.3}` becomes simply
`{foo}`, which allows the fully rendered width of the field to appear
in any formatted result.
Problem: The Deduplicator class combines lines that would result in the same result after applying a format string (with some fields possibly excluded). This doesn't work when the expansion sentinel `+:` is used, because the Deduplication comes before the expansion, so the final maximum field width is not known, and instead the minimm field width is used. This causes unlike lines that differ beyond the minimum width to be combined. Pass `nullify_expansion=True` when copying the input format so that the width is dropped from these fields and deduplication uses the full width for comparison. Fixes flux-framework#6625
Problem: There are no unit tests for OutputFormat.copy(). Add some tests to python/t0024-util.py.
Problem: There is no test that ensures `flux resource status` does not combine dissimilar drain reasons when the reason field has the expandable `+:` prefix sentinel. Add a test to t2354-resource-status.t for this purpose.
Problem: The "show truncation" format string suffix character (`+`) is effectively ignored when the "expandable field" sentinel (`+:`) is used because the field never will be truncaated. However, some format strings in `flux-resource.py` use these together, potentially causing confusion. Remove the `+` suffix on fields using `+:` in the builtin flux-resource format strings.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6638 +/- ##
==========================================
+ Coverage 83.87% 83.88% +0.01%
==========================================
Files 533 533
Lines 88372 88377 +5
==========================================
+ Hits 74122 74137 +15
+ Misses 14250 14240 -10
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes #6625. Expandable width fields (those with the
+:or?+:prefix sentinel) are passed to theDeduplicatorclass before the final maximum width is known, so comparison only occurs up to the minimum width. This was causing drain reasons that differ past the 30 character minimum width to be incorrectly combined.The fix here strips the field widths from expandable fields in the
Deduplicatorso that the entire field is compared.