From b47e5d270cac0700c9d6e8d04e4a32781476011e Mon Sep 17 00:00:00 2001 From: Markus Zolliker Date: Tue, 20 Jun 2023 08:41:11 +0200 Subject: [PATCH] fix frappy.lib.merge_status merge_status should consider that the status text in arguments is already composed. add test for this. Change-Id: Ia000d1e1d8e97a5c2b5ef9d1569cc123232016ae Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/31378 Tested-by: Jenkins Automated Tests Reviewed-by: Alexander Zaft Reviewed-by: Enrico Faulhaber Reviewed-by: Markus Zolliker --- frappy/lib/__init__.py | 13 ++++++++++--- test/test_lib.py | 18 +++++++++++++++++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/frappy/lib/__init__.py b/frappy/lib/__init__.py index 827ac032..a63eebeb 100644 --- a/frappy/lib/__init__.py +++ b/frappy/lib/__init__.py @@ -401,8 +401,15 @@ class UniqueObject: def merge_status(*args): """merge status - the status with biggest code wins - texts matching maximal code are joined with ', ' + for combining stati of different mixins + - the status with biggest code wins + - texts matching maximal code are joined with ', ' + - if texts already contain ', ', it is considered as composed by + individual texts and duplication is avoided. when commas are used + for other purposes, the behaviour might be surprising """ maxcode = max(a[0] for a in args) - return maxcode, ', '.join([a[1] for a in args if a[0] == maxcode and a[1]]) + merged = [a[1] for a in args if a[0] == maxcode and a[1]] + # use dict instead of set for preserving order + merged = {m: True for mm in merged for m in mm.split(', ')} + return maxcode, ', '.join(merged) diff --git a/test/test_lib.py b/test/test_lib.py index 03c09a99..0fb85e00 100644 --- a/test/test_lib.py +++ b/test/test_lib.py @@ -22,7 +22,7 @@ import pytest -from frappy.lib import parse_host_port +from frappy.lib import parse_host_port, merge_status @pytest.mark.parametrize('hostport, defaultport, result', [ @@ -46,3 +46,19 @@ def test_parse_host(hostport, defaultport, result): parse_host_port(hostport, defaultport) else: assert result == parse_host_port(hostport, defaultport) + + +@pytest.mark.parametrize('args, result', [ + ([(100, 'idle'), (200, 'warning')], + (200, 'warning')), + ([(300, 'ramping'), (300, 'within tolerance')], + (300, 'ramping, within tolerance')), + ([(300, 'ramping, within tolerance'), (300, 'within tolerance, slow'), (200, 'warning')], + (300, 'ramping, within tolerance, slow')), + # when a comma is used for other purposes than separating individual status texts, + # the behaviour might not be as desired. However, this case is somewhat constructed. + ([(100, 'blue, yellow is my favorite'), (100, 'white, blue, red is a bad color mix')], + (100, 'blue, yellow is my favorite, white, red is a bad color mix')), +]) +def test_merge_status(args, result): + assert merge_status(*args) == result