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 <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Alexander Zaft <a.zaft@fz-juelich.de>
Reviewed-by: Enrico Faulhaber <enrico.faulhaber@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>

frappy/lib/__init__.py

@@ -401,8 +401,15 @@ class UniqueObject:
 def merge_status(*args):
     """merge status
 
-    the status with biggest code wins
+    for combining stati of different mixins
-    texts matching maximal code are joined with ', '
+    - 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)

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