config: error on invalid configuration

Since the default led priority is no longer 'Blink', the priority now
has to be explicitly defined when using either group priority or led
priority.

If a configuration does not define the priority, the configuration is
invalid and in the yaml case, phosphor-led-manager should fail to build,
in the json case, the process should exit due to the configuration
error.

The config validation has been extracted into it's own file and made
separate from json config parsing.

So every config will go through the same validation even if its been
created via yaml.

Change-Id: Ifda65942b0768d6c0d3b25076f7a1236b46b3d9f
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
diff --git a/manager/config-validator.cpp b/manager/config-validator.cpp
new file mode 100644
index 0000000..05b0157
--- /dev/null
+++ b/manager/config-validator.cpp
@@ -0,0 +1,177 @@
+#include "config-validator.hpp"
+
+#include "grouplayout.hpp"
+#include "ledlayout.hpp"
+
+#include <phosphor-logging/lg2.hpp>
+#include <sdbusplus/bus.hpp>
+
+using namespace phosphor::led;
+
+namespace phosphor
+{
+namespace led
+{
+
+// Priority for a particular LED needs to stay SAME across all groups
+using PriorityMap =
+    std::unordered_map<std::string,
+                       std::optional<phosphor::led::Layout::Action>>;
+
+static bool isUsingGroupPriority(const phosphor::led::GroupMap& ledMap)
+{
+    for (const auto& [_, group] : ledMap)
+    {
+        if (group.priority != 0)
+        {
+            return true;
+        }
+    }
+    return false;
+}
+
+static std::string priorityToString(
+    const std::optional<phosphor::led::Layout::Action>& priority)
+{
+    if (!priority.has_value())
+    {
+        return "none";
+    }
+    switch (priority.value())
+    {
+        case phosphor::led::Layout::Action::Off:
+            return "Off";
+        case phosphor::led::Layout::Action::On:
+            return "On";
+        case phosphor::led::Layout::Action::Blink:
+            return "Blink";
+    }
+    return "?";
+}
+
+/** @brief Validate the Priority of an LED is same across ALL groups
+ *
+ *  @param[in] name - led name member of each group
+ *  @param[in] priority - member priority of each group
+ *  @param[out] priorityMap - std::unordered_map, key:name, value:priority
+ *
+ *  @return
+ */
+void validatePriority(
+    const std::string& name,
+    const std::optional<phosphor::led::Layout::Action>& priority,
+    PriorityMap& priorityMap)
+{
+    auto iter = priorityMap.find(name);
+    if (iter == priorityMap.end())
+    {
+        priorityMap.emplace(name, priority);
+        return;
+    }
+
+    if (iter->second != priority)
+    {
+        throw ConfigValidationException(
+            error::LedPriorityMismatch, "?", name,
+            "Priority of the LED is not same across groups. Old Priority = " +
+                priorityToString(iter->second) +
+                ", New Priority = " + priorityToString(priority));
+    }
+}
+
+static void validateConfigV1GroupForLedPriority(
+    const std::string groupName,
+    const phosphor::led::Layout::GroupLayout& group, PriorityMap& priorityMap)
+{
+    if (group.priority != 0)
+    {
+        throw ConfigValidationException(
+            error::MixedLedAndGroupPriority, groupName,
+            "Cannot mix group priority and led priority");
+    }
+
+    for (const auto& ledAction : group.actionSet)
+    {
+        if (ledAction.priority == std::nullopt)
+        {
+            throw ConfigValidationException(error::MissingLedPriority,
+                                            groupName, ledAction.name,
+                                            "Need valid led priority");
+        }
+
+        // Same LEDs can be part of multiple groups. However, their
+        // priorities across groups need to match.
+        validatePriority(ledAction.name, ledAction.priority, priorityMap);
+    }
+}
+
+static void
+    validateConfigV1ForLedPriority(const phosphor::led::GroupMap& ledMap)
+{
+    PriorityMap priorityMap{};
+    for (const auto& [groupName, group] : ledMap)
+    {
+        validateConfigV1GroupForLedPriority(groupName, group, priorityMap);
+    }
+}
+
+static void validateConfigV1GroupForGroupPriority(
+    const std::string groupName,
+    const phosphor::led::Layout::GroupLayout& group)
+{
+    for (const auto& led : group.actionSet)
+    {
+        if (led.priority != std::nullopt)
+        {
+            throw ConfigValidationException(
+                error::MixedLedAndGroupPriority, groupName, led.name,
+                "Cannot mix group priority and led priority for LED");
+        }
+    }
+
+    if (group.priority == 0)
+    {
+        // group priority 0 is for internal use
+        throw ConfigValidationException(error::InvalidGroupPriority, groupName,
+                                        "Group Priority cannot be 0");
+    }
+}
+
+static void
+    validateConfigV1ForGroupPriority(const phosphor::led::GroupMap& ledMap)
+{
+    std::set<int> groupPriorities;
+    for (const auto& [_, group] : ledMap)
+    {
+        groupPriorities.insert(group.priority);
+    }
+
+    if (groupPriorities.size() != ledMap.size())
+    {
+        throw ConfigValidationException(
+            error::DuplicateGroupPriority,
+            "When using Group Priority, no 2 Groups may have the same priority");
+    }
+
+    for (const auto& [groupName, group] : ledMap)
+    {
+        validateConfigV1GroupForGroupPriority(groupName, group);
+    }
+}
+
+void validateConfigV1(const GroupMap& ledMap)
+{
+    const bool useGroupPriority = isUsingGroupPriority(ledMap);
+
+    if (useGroupPriority)
+    {
+        validateConfigV1ForGroupPriority(ledMap);
+    }
+    else
+    {
+        validateConfigV1ForLedPriority(ledMap);
+    }
+}
+
+} // namespace led
+} // namespace phosphor
diff --git a/manager/config-validator.hpp b/manager/config-validator.hpp
new file mode 100644
index 0000000..5d9ff5f
--- /dev/null
+++ b/manager/config-validator.hpp
@@ -0,0 +1,68 @@
+#include "grouplayout.hpp"
+#include "ledlayout.hpp"
+
+#include <phosphor-logging/lg2.hpp>
+#include <sdbusplus/bus.hpp>
+
+namespace phosphor
+{
+namespace led
+{
+namespace error
+{
+enum ConfigValidationError
+{
+    // An LED has different priorities assigned to it in different groups
+    LedPriorityMismatch,
+
+    // LED priority was needed but not assigned
+    MissingLedPriority,
+
+    // Mixup of the 2 configuration options
+    MixedLedAndGroupPriority,
+
+    // An invalid group priority was assigned
+    InvalidGroupPriority,
+
+    // Group priorities were not unique
+    DuplicateGroupPriority,
+};
+}
+
+class ConfigValidationException : std::runtime_error
+{
+  public:
+    error::ConfigValidationError reason;
+
+    ConfigValidationException(const error::ConfigValidationError& err,
+                              const std::string& msg) :
+        std::runtime_error(msg), reason(err)
+    {
+        lg2::error(msg.c_str());
+    }
+
+    ConfigValidationException(const error::ConfigValidationError& err,
+                              const std::string& groupName,
+                              const std::string& msg) :
+        std::runtime_error(msg), reason(err)
+    {
+        lg2::error("Configuration Validation Error in Group {GROUP}: {MSG}",
+                   "GROUP", groupName, "MSG", msg.c_str());
+    }
+
+    ConfigValidationException(const error::ConfigValidationError& err,
+                              const std::string& groupName,
+                              const std::string& ledName,
+                              const std::string& msg) :
+        std::runtime_error(msg), reason(err)
+    {
+        lg2::error(
+            "Configuration Validation Error in Group {GROUP}, Led {LED}: {MSG}",
+            "GROUP", groupName, "LED", ledName, "MSG", msg.c_str());
+    }
+};
+
+void validateConfigV1(const phosphor::led::GroupMap& ledMap);
+
+} // namespace led
+} // namespace phosphor
diff --git a/manager/json-parser.hpp b/manager/json-parser.hpp
index 8f2832b..30a06a4 100644
--- a/manager/json-parser.hpp
+++ b/manager/json-parser.hpp
@@ -1,5 +1,6 @@
 #include "config.h"
 
+#include "config-validator.hpp"
 #include "grouplayout.hpp"
 #include "json-config.hpp"
 #include "ledlayout.hpp"
@@ -76,59 +77,7 @@
     return phosphor::led::Layout::Action::Blink;
 }
 
-static std::string priorityToString(
-    const std::optional<phosphor::led::Layout::Action>& priority)
-{
-    if (!priority.has_value())
-    {
-        return "none";
-    }
-    switch (priority.value())
-    {
-        case phosphor::led::Layout::Action::Off:
-            return "Off";
-        case phosphor::led::Layout::Action::On:
-            return "On";
-        case phosphor::led::Layout::Action::Blink:
-            return "Blink";
-    }
-    return "?";
-}
-
-/** @brief Validate the Priority of an LED is same across ALL groups
- *
- *  @param[in] name - led name member of each group
- *  @param[in] priority - member priority of each group
- *  @param[out] priorityMap - std::unordered_map, key:name, value:priority
- *
- *  @return
- */
-void validatePriority(
-    const std::string& name,
-    const std::optional<phosphor::led::Layout::Action>& priority,
-    PriorityMap& priorityMap)
-{
-    auto iter = priorityMap.find(name);
-    if (iter == priorityMap.end())
-    {
-        priorityMap.emplace(name, priority);
-        return;
-    }
-
-    if (iter->second != priority)
-    {
-        lg2::error(
-            "Priority of LED is not same across all, Name = {NAME}, Old Priority = {OLD_PRIO}, New Priority = {NEW_PRIO}",
-            "NAME", name, "OLD_PRIO", priorityToString(iter->second),
-            "NEW_PRIO", priorityToString(priority));
-
-        throw std::runtime_error(
-            "Priority of at least one LED is not same across groups");
-    }
-}
-
 static void loadJsonConfigV1GroupMember(const Json& member,
-                                        PriorityMap& priorityMap,
                                         phosphor::led::ActionSet& ledActions)
 {
     auto name = member.value("Name", "");
@@ -144,18 +93,13 @@
         priority = getAction(priorityStr);
     }
 
-    // Same LEDs can be part of multiple groups. However, their
-    // priorities across groups need to match.
-    validatePriority(name, priority, priorityMap);
-
     phosphor::led::Layout::LedAction ledAction{name, action, dutyOn, period,
                                                priority};
     ledActions.emplace(ledAction);
 }
 
 static void loadJsonConfigV1Group(const Json& entry,
-                                  phosphor::led::GroupMap& ledMap,
-                                  PriorityMap& priorityMap)
+                                  phosphor::led::GroupMap& ledMap)
 {
     const Json empty{};
 
@@ -166,15 +110,16 @@
     tmpPath /= groupName;
     auto objpath = tmpPath.string();
     auto members = entry.value("members", empty);
-    int priority = entry.value("Priority", 0);
 
     lg2::debug("config for '{GROUP}'", "GROUP", groupName);
 
+    int priority = entry.value("Priority", 0);
+
     phosphor::led::ActionSet ledActions{};
     phosphor::led::Layout::GroupLayout groupLayout{};
     for (const auto& member : members)
     {
-        loadJsonConfigV1GroupMember(member, priorityMap, ledActions);
+        loadJsonConfigV1GroupMember(member, ledActions);
     }
 
     // Generated an std::unordered_map of LedGroupNames to std::set of LEDs
@@ -192,7 +137,6 @@
 const phosphor::led::GroupMap loadJsonConfigV1(const Json& json)
 {
     phosphor::led::GroupMap ledMap{};
-    PriorityMap priorityMap{};
 
     // define the default JSON as empty
     const Json empty{};
@@ -200,7 +144,7 @@
 
     for (const auto& entry : leds)
     {
-        loadJsonConfigV1Group(entry, ledMap, priorityMap);
+        loadJsonConfigV1Group(entry, ledMap);
     }
 
     return ledMap;
diff --git a/manager/led-main.cpp b/manager/led-main.cpp
index a18c793..6d1ef82 100644
--- a/manager/led-main.cpp
+++ b/manager/led-main.cpp
@@ -42,6 +42,8 @@
     auto systemLedMap = getSystemLedMap(configFile);
 #endif
 
+    phosphor::led::validateConfigV1(systemLedMap);
+
     /** @brief Group manager object */
     phosphor::led::Manager manager(bus, systemLedMap, event);
 
diff --git a/manager/meson.build b/manager/meson.build
index e2452bd..0c97649 100644
--- a/manager/meson.build
+++ b/manager/meson.build
@@ -4,6 +4,7 @@
     'manager.cpp',
     'serialize.cpp',
     '../utils.cpp',
+    'config-validator.cpp',
 ]
 
 if get_option('use-json').disabled()
diff --git a/scripts/parse_led.py b/scripts/parse_led.py
index eac764c..5724e6d 100755
--- a/scripts/parse_led.py
+++ b/scripts/parse_led.py
@@ -6,16 +6,30 @@
 from inflection import underscore
 
 
-def check_led_priority(led_name, value, priority_dict):
+def config_error(ofile, led_name, group_name, message):
+    ofile.close()
+    os.remove(ofile.name)
+    raise ValueError(
+        "Invalid Configuration for LED ["
+        + led_name
+        + "] in Group ["
+        + group_name
+        + "]: "
+        + message
+    )
+
+
+def check_led_priority(led_name, group, value, priority_dict):
 
     if led_name in priority_dict:
         if value != priority_dict[led_name]:
             # Priority for a particular LED needs to stay SAME
             # across all groups
-            ofile.close()
-            os.remove(ofile.name)
-            raise ValueError(
-                "Priority for [" + led_name + "] is NOT same across all groups"
+            config_error(
+                ofile,
+                led_name,
+                group,
+                "Priority is NOT same across all groups",
             )
     else:
         priority_dict[led_name] = value
@@ -23,20 +37,41 @@
     return 0
 
 
-def generate_file_single_led(ifile, led_name, list_dict, priority_dict, ofile):
+def led_action_literal(action):
+    if action == "":
+        return "std::nullopt"
 
-    value = list_dict.get("Priority")
+    return "phosphor::led::Layout::Action::" + str(action)
 
-    check_led_priority(led_name, value, priority_dict)
 
-    action = "phosphor::led::Layout::Action::" + str(
-        list_dict.get("Action", "Off")
-    )
+def generate_file_single_led(
+    ifile, led_name, list_dict, priority_dict, ofile, has_group_priority, group
+):
+
+    has_led_priority = "Priority" in list_dict
+
+    if has_group_priority and has_led_priority:
+        config_error(
+            ofile,
+            led_name,
+            group,
+            "cannot mix group priority with led priority",
+        )
+
+    if (not has_group_priority) and (not has_led_priority):
+        config_error(
+            ofile, led_name, group, "no group priority or led priority defined"
+        )
+
+    led_priority = list_dict.get("Priority", "")
+
+    if has_led_priority:
+        check_led_priority(led_name, group, led_priority, priority_dict)
+
+    action = led_action_literal(list_dict.get("Action", "Off"))
     dutyOn = str(list_dict.get("DutyOn", 50))
     period = str(list_dict.get("Period", 0))
-    priority = "phosphor::led::Layout::Action::" + str(
-        list_dict.get("Priority", "Blink")
-    )
+    priority = led_action_literal(led_priority)
 
     ofile.write('        {"' + underscore(led_name) + '",')
     ofile.write(action + ",")
@@ -75,7 +110,13 @@
 
     for led_name, list_dict in list(led_dict.items()):
         generate_file_single_led(
-            ifile, led_name, list_dict, priority_dict, ofile
+            ifile,
+            led_name,
+            list_dict,
+            priority_dict,
+            ofile,
+            has_group_priority,
+            group,
         )
 
     ofile.write("   }}},\n")
diff --git a/test/config/led-group-config-malformed.json b/test/config/led-group-config-malformed.json
deleted file mode 100644
index 3e7d668..0000000
--- a/test/config/led-group-config-malformed.json
+++ /dev/null
@@ -1,40 +0,0 @@
-{
-    "leds": [
-        {
-            "group": "bmc_booted"
-            "members": [
-                {
-                    "Name": "heartbeat",
-                    "Action": "On"
-                }
-            ]
-        },
-        {
-            "group": "power_on",
-            "members": [
-                {
-                    "Name": "power",
-                    "Action": "On",
-                    "Priority": "On"
-                }
-            ]
-        },
-        {
-            "group": "enclosure_identify",
-            "members": [
-                {
-                    "Name": "front_id",
-                    "Action": "Blink",
-                    "DutyOn": 50,
-                    "Period": 1000
-                },
-                {
-                    "Name": "rear_id",
-                    "Action": "Blink",
-                    "DutyOn": 50,
-                    "Period": 1000
-                }
-            ]
-        }
-    ]
-}
\ No newline at end of file
diff --git a/test/config/led-group-config.json b/test/config/led-group-config.json
index 0a4ccb9..f2ce5d7 100644
--- a/test/config/led-group-config.json
+++ b/test/config/led-group-config.json
@@ -5,7 +5,8 @@
             "members": [
                 {
                     "Name": "heartbeat",
-                    "Action": "On"
+                    "Action": "On",
+                    "Priority": "Blink"
                 }
             ]
         },
@@ -36,13 +37,15 @@
                     "Name": "front_id",
                     "Action": "Blink",
                     "DutyOn": 50,
-                    "Period": 1000
+                    "Period": 1000,
+                    "Priority": "Blink"
                 },
                 {
                     "Name": "rear_id",
                     "Action": "Blink",
                     "DutyOn": 50,
-                    "Period": 1000
+                    "Period": 1000,
+                    "Priority": "Blink"
                 }
             ]
         }
diff --git a/test/config/test-group-priority.yaml b/test/config/test-group-priority.yaml
index ca5005f..1acfbb6 100644
--- a/test/config/test-group-priority.yaml
+++ b/test/config/test-group-priority.yaml
@@ -1,10 +1,13 @@
 group1:
+    Priority: 1
     led1:
         Action: "On"
 group2:
+    Priority: 2
     led1:
         Action: "Off"
 group3:
+    Priority: 3
     led1:
         Action: "Blink"
         DutyOn: 50
diff --git a/test/meson.build b/test/meson.build
index 010f7f4..4aa6632 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -26,6 +26,7 @@
 
 test_sources = [
   '../manager/manager.cpp',
+  '../manager/config-validator.cpp',
   '../utils.cpp',
 ]
 
@@ -50,6 +51,7 @@
   'utest-group-priority.cpp',
   'utest-led-yaml-group-priority.cpp',
   'utest-led-yaml-led-priority.cpp',
+  'utest-config-validator.cpp'
 ]
 if get_option('persistent-led-asserted').allowed()
   test_sources += [
diff --git a/test/utest-config-validator.cpp b/test/utest-config-validator.cpp
new file mode 100644
index 0000000..1eda517
--- /dev/null
+++ b/test/utest-config-validator.cpp
@@ -0,0 +1,143 @@
+#include "config-validator.hpp"
+#include "grouplayout.hpp"
+#include "ledlayout.hpp"
+
+#include <gtest/gtest.h>
+
+using namespace phosphor::led;
+
+static void assertValidationException(const GroupMap& ledMap,
+                                      error::ConfigValidationError err)
+{
+    try
+    {
+        validateConfigV1(ledMap);
+        ASSERT_FALSE(true);
+    }
+    catch (ConfigValidationException& e)
+    {
+        ASSERT_EQ(e.reason, err);
+    }
+}
+
+TEST(validateConfig, testGoodPathLedPriority)
+{
+    ActionSet group1ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, Layout::Action::On},
+        {"led2", Layout::Action::On, 0, 0, Layout::Action::Blink},
+    };
+    Layout::GroupLayout group1 = {0, group1ActionSet};
+    GroupMap ledMap = {{"group1", group1}};
+
+    validateConfigV1(ledMap);
+}
+
+TEST(validateConfig, testGoodPathGroupPriority)
+{
+    ActionSet group1ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, std::nullopt},
+        {"led2", Layout::Action::On, 0, 0, std::nullopt},
+    };
+    ActionSet group2ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, std::nullopt},
+        {"led2", Layout::Action::On, 0, 0, std::nullopt},
+    };
+    Layout::GroupLayout group1 = {1, group1ActionSet};
+    Layout::GroupLayout group2 = {2, group1ActionSet};
+    GroupMap ledMap = {
+        {"group1", group1},
+        {"group2", group2},
+    };
+
+    validateConfigV1(ledMap);
+}
+
+TEST(validateConfig, testLedPriorityMismatch)
+{
+    ActionSet group1ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, Layout::Action::On},
+    };
+    ActionSet group2ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, Layout::Action::Off},
+    };
+    Layout::GroupLayout group1 = {0, group1ActionSet};
+    Layout::GroupLayout group2 = {0, group2ActionSet};
+    GroupMap ledMap = {
+        {"group1", group1},
+        {"group2", group2},
+    };
+
+    assertValidationException(ledMap, error::LedPriorityMismatch);
+}
+
+TEST(validateConfig, testMissingLedPriority)
+{
+    ActionSet group1ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, Layout::Action::On},
+    };
+    ActionSet group2ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, std::nullopt},
+    };
+    Layout::GroupLayout group1 = {0, group1ActionSet};
+    Layout::GroupLayout group2 = {0, group2ActionSet};
+    GroupMap ledMap = {
+        {"group1", group1},
+        {"group2", group2},
+    };
+
+    assertValidationException(ledMap, error::MissingLedPriority);
+}
+
+TEST(validateConfig, testMixedLedAndGroupPriority)
+{
+    ActionSet group1ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, Layout::Action::On},
+    };
+    ActionSet group2ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, Layout::Action::On},
+    };
+    Layout::GroupLayout group1 = {0, group1ActionSet};
+    Layout::GroupLayout group2 = {1, group2ActionSet};
+    GroupMap ledMap = {
+        {"group1", group1},
+        {"group2", group2},
+    };
+
+    assertValidationException(ledMap, error::MixedLedAndGroupPriority);
+}
+
+TEST(validateConfig, testInvalidGroupPriority)
+{
+    ActionSet group1ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, std::nullopt},
+    };
+    ActionSet group2ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, std::nullopt},
+    };
+    Layout::GroupLayout group1 = {0, group1ActionSet};
+    Layout::GroupLayout group2 = {1, group2ActionSet};
+    GroupMap ledMap = {
+        {"group1", group1},
+        {"group2", group2},
+    };
+
+    assertValidationException(ledMap, error::InvalidGroupPriority);
+}
+
+TEST(validateConfig, testDuplicateGroupPriority)
+{
+    ActionSet group1ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, std::nullopt},
+    };
+    ActionSet group2ActionSet = {
+        {"led1", Layout::Action::On, 0, 0, std::nullopt},
+    };
+    Layout::GroupLayout group1 = {1, group1ActionSet};
+    Layout::GroupLayout group2 = {1, group2ActionSet};
+    GroupMap ledMap = {
+        {"group1", group1},
+        {"group2", group2},
+    };
+
+    assertValidationException(ledMap, error::DuplicateGroupPriority);
+}
diff --git a/test/utest-led-json.cpp b/test/utest-led-json.cpp
index fa9161b..318c9bc 100644
--- a/test/utest-led-json.cpp
+++ b/test/utest-led-json.cpp
@@ -29,7 +29,7 @@
         ASSERT_EQ(group.action, phosphor::led::Layout::Action::On);
         ASSERT_EQ(group.dutyOn, 50);
         ASSERT_EQ(group.period, 0);
-        ASSERT_EQ(group.priority, std::nullopt);
+        ASSERT_EQ(group.priority, phosphor::led::Layout::Action::Blink);
     }
 
     for (const auto& group : powerOnActions)
@@ -54,14 +54,14 @@
             ASSERT_EQ(group.action, phosphor::led::Layout::Action::Blink);
             ASSERT_EQ(group.dutyOn, 50);
             ASSERT_EQ(group.period, 1000);
-            ASSERT_EQ(group.priority, std::nullopt);
+            ASSERT_EQ(group.priority, phosphor::led::Layout::Action::Blink);
         }
         else if (group.name == "rear_id")
         {
             ASSERT_EQ(group.action, phosphor::led::Layout::Action::Blink);
             ASSERT_EQ(group.dutyOn, 50);
             ASSERT_EQ(group.period, 1000);
-            ASSERT_EQ(group.priority, std::nullopt);
+            ASSERT_EQ(group.priority, phosphor::led::Layout::Action::Blink);
         }
         else
         {
@@ -69,35 +69,3 @@
         }
     }
 }
-
-TEST(loadJsonConfig, testBadPath)
-{
-    static constexpr auto jsonPath = "config/led-group-config-malformed.json";
-    ASSERT_THROW(loadJsonConfig(jsonPath), std::exception);
-}
-
-TEST(validatePriority, testGoodPriority)
-{
-    PriorityMap priorityMap{};
-    validatePriority("heartbeat", phosphor::led::Layout::Action::Blink,
-                     priorityMap);
-    validatePriority("power", phosphor::led::Layout::Action::On, priorityMap);
-
-    ASSERT_EQ(priorityMap.at("heartbeat"),
-              phosphor::led::Layout::Action::Blink);
-    ASSERT_EQ(priorityMap.at("power"), phosphor::led::Layout::Action::On);
-}
-
-TEST(validatePriority, testBadPriority)
-{
-    PriorityMap priorityMap{};
-    validatePriority("heartbeat", phosphor::led::Layout::Action::Blink,
-                     priorityMap);
-
-    ASSERT_EQ(priorityMap.at("heartbeat"),
-              phosphor::led::Layout::Action::Blink);
-    ASSERT_THROW(validatePriority("heartbeat",
-                                  phosphor::led::Layout::Action::On,
-                                  priorityMap),
-                 std::runtime_error);
-}
diff --git a/test/utest-led-yaml-group-priority.cpp b/test/utest-led-yaml-group-priority.cpp
index e48a199..b257c17 100644
--- a/test/utest-led-yaml-group-priority.cpp
+++ b/test/utest-led-yaml-group-priority.cpp
@@ -26,13 +26,14 @@
 
     phosphor::led::Layout::GroupLayout group = systemLedMap.at(groupPath);
 
-    EXPECT_EQ(group.priority, 0);
+    EXPECT_EQ(group.priority, 1);
     EXPECT_EQ(group.actionSet.size(), 1);
 
     for (auto& led : group.actionSet)
     {
         EXPECT_EQ(led.name, "led1");
         EXPECT_EQ(led.action, Action::On);
+        EXPECT_EQ(led.priority, std::nullopt);
     }
 }
 
@@ -43,13 +44,14 @@
 
     phosphor::led::Layout::GroupLayout group = systemLedMap.at(groupPath);
 
-    EXPECT_EQ(group.priority, 0);
+    EXPECT_EQ(group.priority, 2);
     EXPECT_EQ(group.actionSet.size(), 1);
 
     for (auto& led : group.actionSet)
     {
         EXPECT_EQ(led.name, "led1");
         EXPECT_EQ(led.action, Action::Off);
+        EXPECT_EQ(led.priority, std::nullopt);
     }
 }
 
@@ -60,7 +62,7 @@
 
     phosphor::led::Layout::GroupLayout group = systemLedMap.at(groupPath);
 
-    EXPECT_EQ(group.priority, 0);
+    EXPECT_EQ(group.priority, 3);
     EXPECT_EQ(group.actionSet.size(), 1);
 
     for (auto& led : group.actionSet)
@@ -69,6 +71,7 @@
         EXPECT_EQ(led.action, Action::Blink);
         EXPECT_EQ(led.dutyOn, 50);
         EXPECT_EQ(led.period, 1000);
+        EXPECT_EQ(led.priority, std::nullopt);
     }
 }
 
@@ -88,11 +91,13 @@
         if (led.name == "led1")
         {
             EXPECT_EQ(led.action, Action::On);
+            EXPECT_EQ(led.priority, std::nullopt);
             found++;
         }
         if (led.name == "led2")
         {
             EXPECT_EQ(led.action, Action::Off);
+            EXPECT_EQ(led.priority, std::nullopt);
             found++;
         }
     }