From f0601596b4b88bc085138f0c4b0296c94863aa02 Mon Sep 17 00:00:00 2001
From: Teemu R <tpr@iki.fi>
Date: Wed, 8 Mar 2023 04:40:40 +0100
Subject: [PATCH] Pull 'unit' up to the descriptor base class (#1761)

As this is used by all but actions, it belongs to the base class. This
also adds tests to the descriptors that were previously missing.
---
 miio/descriptors.py            |   7 +--
 miio/tests/test_descriptors.py | 101 +++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 4 deletions(-)
 create mode 100644 miio/tests/test_descriptors.py

diff --git a/miio/descriptors.py b/miio/descriptors.py
index 73c4fcd5b..2687f1ef6 100644
--- a/miio/descriptors.py
+++ b/miio/descriptors.py
@@ -36,7 +36,6 @@ def __str__(self):
         s += "r" if self & AccessFlags.Read else "-"
         s += "w" if self & AccessFlags.Write else "-"
         s += "x" if self & AccessFlags.Execute else "-"
-        s += ""
         return s
 
 
@@ -50,12 +49,14 @@ class Descriptor:
     name: str
     #: Type of the property, if applicable.
     type: Optional[type] = None
+    #: Unit of the property, if applicable.
+    unit: Optional[str] = None
     #: Name of the attribute in the status container that contains the value, if applicable.
     status_attribute: Optional[str] = None
     #: Additional data related to this descriptor.
     extras: Dict = attr.ib(factory=dict, repr=False)
     #: Access flags (read, write, execute) for the described item.
-    access: AccessFlags = attr.ib(default=AccessFlags.Read | AccessFlags.Write)
+    access: AccessFlags = attr.ib(default=AccessFlags(0))
 
 
 @attr.s(auto_attribs=True)
@@ -94,8 +95,6 @@ class PropertyDescriptor(Descriptor):
     status_attribute: str
     #: Sensors are read-only and settings are (usually) read-write.
     access: AccessFlags = attr.ib(default=AccessFlags.Read)
-    #: Optional human-readable unit of the property.
-    unit: Optional[str] = None
 
     #: Constraint type defining the allowed values for an integer property.
     constraint: PropertyConstraint = attr.ib(default=PropertyConstraint.Unset)
diff --git a/miio/tests/test_descriptors.py b/miio/tests/test_descriptors.py
new file mode 100644
index 000000000..b9bc663a0
--- /dev/null
+++ b/miio/tests/test_descriptors.py
@@ -0,0 +1,101 @@
+from enum import Enum
+
+import pytest
+
+from miio.descriptors import (
+    AccessFlags,
+    ActionDescriptor,
+    Descriptor,
+    EnumDescriptor,
+    PropertyConstraint,
+    PropertyDescriptor,
+)
+
+COMMON_FIELDS = {
+    "id": "test",
+    "name": "Test",
+    "type": int,
+    "status_attribute": "test",
+    "unit": "unit",
+    "extras": {"test": "test"},
+}
+
+
+def test_accessflags():
+    """Test that accessflags str representation is correct."""
+    assert str(AccessFlags(AccessFlags.Read)) == "r--"
+    assert str(AccessFlags(AccessFlags.Write)) == "-w-"
+    assert str(AccessFlags(AccessFlags.Execute)) == "--x"
+    assert str(AccessFlags(AccessFlags.Read | AccessFlags.Write)) == "rw-"
+
+
+@pytest.mark.parametrize(
+    ("class_", "access"),
+    [
+        pytest.param(Descriptor, AccessFlags(0), id="base class (no access)"),
+        pytest.param(ActionDescriptor, AccessFlags.Execute, id="action (execute)"),
+        pytest.param(
+            PropertyDescriptor, AccessFlags.Read, id="regular property (read)"
+        ),
+    ],
+)
+def test_descriptor(class_, access):
+    """Test that the common descriptor has the expected API."""
+    desc = class_(**COMMON_FIELDS)
+    assert desc.id == "test"
+    assert desc.name == "Test"
+    assert desc.type == int
+    assert desc.status_attribute == "test"
+    assert desc.extras == {"test": "test"}
+    assert desc.access == access
+
+
+def test_actiondescriptor():
+    """Test that an action descriptor has the expected API."""
+    desc = ActionDescriptor(id="test", name="Test", extras={"test": "test"})
+    assert desc.id == "test"
+    assert desc.name == "Test"
+    assert desc.method_name is None
+    assert desc.type is None
+    assert desc.status_attribute is None
+    assert desc.inputs is None
+    assert desc.extras == {"test": "test"}
+    assert desc.access == AccessFlags.Execute
+
+
+def test_propertydescriptor():
+    """Test that a property descriptor has the expected API."""
+    desc = PropertyDescriptor(
+        id="test",
+        name="Test",
+        type=int,
+        status_attribute="test",
+        unit="unit",
+        extras={"test": "test"},
+    )
+    assert desc.id == "test"
+    assert desc.name == "Test"
+    assert desc.type == int
+    assert desc.status_attribute == "test"
+    assert desc.unit == "unit"
+    assert desc.extras == {"test": "test"}
+    assert desc.access == AccessFlags.Read
+
+
+def test_enumdescriptor():
+    """Test that an enum descriptor has the expected API."""
+
+    class TestChoices(Enum):
+        One = 1
+        Two = 2
+
+    desc = EnumDescriptor(**COMMON_FIELDS, choices=TestChoices)
+    assert desc.id == "test"
+    assert desc.name == "Test"
+    assert desc.type == int
+    assert desc.status_attribute == "test"
+    assert desc.unit == "unit"
+    assert desc.extras == {"test": "test"}
+    assert desc.access == AccessFlags.Read
+    assert desc.constraint == PropertyConstraint.Choice
+    assert desc.choices == TestChoices