Adding support for supressing UNKNOWN results via --no-unknown#63
Adding support for supressing UNKNOWN results via --no-unknown#63mikenowak wants to merge 1 commit intotimdaman:masterfrom mikenowak:no_unknown
Conversation
timdaman
left a comment
There was a problem hiding this comment.
Thank for the contribution! I found a couple issues but I think they are both are fairly simple though if you haven't used pytest before it could take a few minutes longer.
| filtered_messages = [message for message in messages if not message.startswith('UNKNOWN: ')] | ||
| if len(filtered_messages) == 0: | ||
| messages_concat = 'OK' | ||
| if no_ok: |
There was a problem hiding this comment.
If the user passes --no_ok --no_unknown
UNKNOWN will be added to the output even if one was not removed.
|
|
||
|
|
||
| @pytest.mark.parametrize("messages, perf_data, no_ok, no_performance, expected", ( | ||
| @pytest.mark.parametrize("messages, perf_data, no_ok, no_unknown, no_performance, expected", ( |
There was a problem hiding this comment.
You added a test parameter but you didn't update the data to match it. Have you used parameterized pytest tests before? It is pretty simple (and also a little ugly).
The string is a ordered list of names for all of the values in each of the tuples that follow. Each tuple in the list describes the setup of a single test case. For example
def test_func(one, two)
...
This executes does something like this
for params in ((1,2),(11,22)):
one, two = params
test_func(one=one, two=two)
Can you add the missing entries to the tuples and also add tuples that exercise the code you are changing.
Don't worry if you break something, I can help you out. Also my dev documentation should help you get the test running on your machine.
|
Hi, I'm closing this for now. Also I think the flag and the use-case could probably solved more generically, e.g. with the existing |
I have a use case where a bunch of containers that I query via --containers all do not have healthchecks defined, this results in an UNKNOWN return. This PR adds support for supressing these messages.
For the test class, I am not entirely sure if I am doing this right, so any feedback/corrections welcome.