Sort dispatched alerts by job+instance in the correct order (#1178)

[tzz]

Alert order was wrongly implemented, trivial change. Followup fix to #1234

[tiagoapimenta]

@tzz Don't you believe you should switch the true/false on lines 433 and 437 as well?

[tzz]

@tiagoapimenta yes you're right. Fixed. This really should have an acceptance test to avoid the hassle but I don't know how to write it.

[simonpasquier]

This really should have an acceptance test to avoid the hassle but I don't know how to write it.

AFAICT the existing sort algorithm on types.Alert is only used by dispatch/dispatch_test.go.

alertmanager/types/types.go

Lines 269 to 271 in cecfe5b

	func (as AlertSlice) Less(i, j int) bool { return as[i].UpdatedAt.Before(as[j].UpdatedAt) }
	func (as AlertSlice) Swap(i, j int) { as[i], as[j] = as[j], as[i] }
	func (as AlertSlice) Len() int { return len(as) }

I guess that you can replace it by what you have implemented in dispatch/dispatch.go. Then you can add the unit tests to types/types_test.go which is simpler than doing it in dispatch/dispatch_test.go.

[tzz]

@simonpasquier thanks for the advice. Is the test a blocker for this PR or can we add it later (so it won't block the 0.15 release)? I definitely can't work on it in the next 2 weeks.

[simonpasquier]

I would rather have some unit tests because it may surface other sorting problems. I could give it a try if you don't mind.

[tzz]

Please do! I would really appreciate it.

[mxinden]

#1349 included this patch with additional tests. I will close here. @tzz please feel free to reopen in case I am missing something.

Thanks a lot for the fix!