Refactor result package#152
Conversation
This patch removes some parts of the overall object in favor of putting everything into subcheck objects.
martialblog
left a comment
martialblog
left a comment
There was a problem hiding this comment.
Very nice.
Note:
if state < check.OK || state > check.Unknown {
return errors.New("Default State is not a valid result state. Got " + state.String() + " which is not valid")
}
We can probably remove this, since we now have proper types.
done |
|
sadly this conflicts with the recent changes on |
This patch removes the "Summary" attribute from the overall object and replaces the functionality with a generated summary. If the overall state is OK, the summary is either a user given string from a new "OKSummary" attribute or auto generated (as previously) with just a small statistics over the states of all PartialResults. If the overall state is NOT OK, the summary is fetched from the worst-deepest-first PartialResult Output.
| } | ||
| } | ||
|
|
||
| func Compare(a Status, b Status) int { |
There was a problem hiding this comment.
Like the idea.
Needs godoc and a why doesn't it return a Status?
Also the function should be fully unittested and have an Example
There was a problem hiding this comment.
Why should it return a Status?
I thought of the classic C style compare functions you can throw into qsort (3).
Yes, more tests and an example are a good idea.
| // String returns the status and output of the PartialResult | ||
| func (s *PartialResult) String() string { | ||
| return fmt.Sprintf("[%s] %s", s.GetStatus(), s.Output) | ||
| OKSummary string |
There was a problem hiding this comment.
Needs a small comment what the OKSummary is and what it does
| } | ||
|
|
||
| func (o *Overall) getGenericSummary() string { | ||
| // OK state, but no Ok_summary set |
There was a problem hiding this comment.
Not sure what the comment is trying to say.
There was a problem hiding this comment.
hm, leftover from moving stuff around
| result := "" | ||
|
|
||
| for _, partRes := range s.PartialResults { | ||
| if check.Compare(worstState, partRes.GetStatus()) > 0 { |
There was a problem hiding this comment.
I think this is the other way around?
func TestGetSummary_Worststate(t *testing.T) {
o := Overall{}
o.Add(check.Warning, "WARN1")
o.Add(check.Critical, "CRIT")
o.Add(check.Warning, "WARN2")
output := o.GetOutput() // Should be CRIT
t.Log(output)
}
=== RUN TestGetSummary_Worststate
overall_test.go:528: WARN2
\_ [WARNING] WARN1
\_ [CRITICAL] CRIT
\_ [WARNING] WARN2
There was a problem hiding this comment.
We need more unittests here as well
There was a problem hiding this comment.
That's an example you just invented?
2 participants
This patch removes some parts of the overall object in favor of putting everything into subcheck objects.
See #151