Replace usage of CONFIG_BUILD_FOR_HOST_UNIT_TEST for IM/DM validation…

… checker (project-chip#35319) * The flag of CONFIG_BUILD_FOR_HOST_UNIT_TEST is not actually tied to unit testing. Implement a separate flag to control if we crash on errors for IM/DM checks or not. * Update src/app/common_flags.gni Co-authored-by: Boris Zbarsky <[email protected]> * make the flag name singular --------- Co-authored-by: Andrei Litvin <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>
shgutte · Sep 10, 2024 · ce4845f · ce4845f
1 parent d10ab8c
commit ce4845f
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 13 deletions.
diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
@@ -75,7 +75,7 @@ jobs:
               with:
                   languages: "cpp"
             - name: Setup Build
-              run: scripts/build/gn_gen.sh --args="chip_config_memory_debug_checks=true chip_config_memory_debug_dmalloc=false"
+              run: scripts/build/gn_gen.sh --args="chip_config_memory_debug_checks=true chip_config_memory_debug_dmalloc=false chip_data_model_check_die_on_failure=true"
             - name: Run Build
               run: scripts/run_in_build_env.sh "ninja -C ./out"
             - name: Run Tests
@@ -179,7 +179,7 @@ jobs:
                   scripts/run_in_build_env.sh "ninja -C ./out/$BUILD_TYPE"
             - name: Setup Build, Run Build and Run Tests
               run: |
-                  BUILD_TYPE=gcc_release scripts/build/gn_gen.sh --args="is_debug=false"
+                  BUILD_TYPE=gcc_release scripts/build/gn_gen.sh --args="is_debug=false chip_data_model_check_die_on_failure=true"
                   scripts/run_in_build_env.sh "ninja -C ./out/gcc_release"
                   BUILD_TYPE=gcc_release scripts/tests/gn_tests.sh
             - name: Clean output
@@ -197,7 +197,7 @@ jobs:
                       esac
 
                       rm -rf ./out/sanitizers
-                      BUILD_TYPE=sanitizers scripts/build/gn_gen.sh --args="$GN_ARGS" --export-compile-commands
+                      BUILD_TYPE=sanitizers scripts/build/gn_gen.sh --args="$GN_ARGS chip_data_model_check_die_on_failure=true" --export-compile-commands
                       BUILD_TYPE=sanitizers scripts/tests/gn_tests.sh
                   done
             - name: Ensure codegen is done for sanitize
@@ -308,7 +308,7 @@ jobs:
 
             - name: Setup Build, Run Build and Run Tests
               run: |
-                  scripts/build/gn_gen.sh --args="enable_rtti=true chip_config_memory_debug_checks=false chip_config_memory_debug_dmalloc=false chip_generate_link_map_file=false"
+                  scripts/build/gn_gen.sh --args="enable_rtti=true chip_config_memory_debug_checks=false chip_config_memory_debug_dmalloc=false chip_generate_link_map_file=false chip_data_model_check_die_on_failure=true"
                   scripts/run_in_build_env.sh "ninja -C ./out"
                   scripts/tests/gn_tests.sh
             - name: Setup test python environment
@@ -415,7 +415,7 @@ jobs:
                           # clang.
                           "default") GN_ARGS='target_os="all" is_asan=true enable_host_clang_build=false';;
                       esac
-                      BUILD_TYPE=$BUILD_TYPE scripts/build/gn_gen.sh --args="$GN_ARGS" --export-compile-commands
+                      BUILD_TYPE=$BUILD_TYPE scripts/build/gn_gen.sh --args="$GN_ARGS chip_data_model_check_die_on_failure=true" --export-compile-commands
                       scripts/run_in_build_env.sh "ninja -C ./out/$BUILD_TYPE"
                       BUILD_TYPE=$BUILD_TYPE scripts/tests/gn_tests.sh
                   done

diff --git a/.github/workflows/unit_integration_test.yaml b/.github/workflows/unit_integration_test.yaml
@@ -73,7 +73,7 @@ jobs:
                      *) ;;
                   esac
 
-                  scripts/build/gn_gen.sh --args="$GN_ARGS"
+                  scripts/build/gn_gen.sh --args="$GN_ARGS chip_data_model_check_die_on_failure=true"
             - name: Run Build
               run: scripts/run_in_build_env.sh "ninja -C out/$BUILD_TYPE"
             - name: Run Tests

diff --git a/scripts/build/builders/host.py b/scripts/build/builders/host.py
@@ -413,6 +413,7 @@ def __init__(self, root, runner, app: HostApp, board=HostBoard.NATIVE,
 
         if app == HostApp.TESTS:
             self.extra_gn_options.append('chip_build_tests=true')
+            self.extra_gn_options.append('chip_data_model_check_die_on_failure=true')
             self.build_command = 'check'
 
         if app == HostApp.EFR32_TEST_RUNNER:

diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
@@ -78,6 +78,7 @@ buildconfig_header("app_buildconfig") {
     "NON_SPEC_COMPLIANT_OTA_ACTION_DELAY_FLOOR=${non_spec_compliant_ota_action_delay_floor}",
     "CHIP_DEVICE_CONFIG_DYNAMIC_SERVER=${chip_build_controller_dynamic_server}",
     "CHIP_CONFIG_ENABLE_BUSY_HANDLING_FOR_OPERATIONAL_SESSION_SETUP=${chip_enable_busy_handling_for_operational_session_setup}",
+    "CHIP_CONFIG_DATA_MODEL_CHECK_DIE_ON_FAILURE=${chip_data_model_check_die_on_failure}",
   ]
 
   if (chip_use_data_model_interface == "disabled") {

diff --git a/src/app/common_flags.gni b/src/app/common_flags.gni
@@ -27,10 +27,14 @@ declare_args() {
   #   - check:    runs BOTH datamodel and non-data-model (if possible) functionality and compares results
   #   - enabled:  runs only the data model interface (does not use the legacy code)
   if (current_os == "linux") {
-    # "check" is broken, see https://github.com/project-chip/connectedhomeip/issues/35306
-    #chip_use_data_model_interface = "check"
-    chip_use_data_model_interface = "disabled"
+    chip_use_data_model_interface = "check"
   } else {
     chip_use_data_model_interface = "disabled"
   }
+
+  # Whether we call `chipDie` on DM `check` errors
+  #
+  # If/once the chip_use_data_model_interface flag is removed or does not support
+  # a `check` option, this should alwo be removed
+  chip_data_model_check_die_on_failure = false
 }
diff --git a/src/app/reporting/Read-Checked.cpp b/src/app/reporting/Read-Checked.cpp
@@ -96,7 +96,7 @@ ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataModel, const Ac
         // Make unit tests strict; otherwise allow it with potentially odd mismatch errors
         // (in which case logs will be odd, however we also expect Checked versions to only
         // run for a short period until we switch over to either ember or DM completely).
-#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
+#if CHIP_CONFIG_DATA_MODEL_CHECK_DIE_ON_FAILURE
         chipDie();
 #endif
     }
@@ -119,7 +119,7 @@ ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataModel, const Ac
     {
         ChipLogError(Test, "Different written length: %" PRIu32 " (Ember) vs %" PRIu32 " (DataModel)", lengthWrittenEmber,
                      reportBuilder.GetWriter()->GetLengthWritten());
-#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
+#if CHIP_CONFIG_DATA_MODEL_CHECK_DIE_ON_FAILURE
         chipDie();
 #endif
     }
@@ -138,7 +138,7 @@ ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataModel, const Ac
                 ChipLogError(Test, "Different partial data");
                 // NOTE: die on unit tests only, since partial data size may differ across
                 //       time-dependent data (very rarely because fast code, but still possible)
-#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
+#if CHIP_CONFIG_DATA_MODEL_CHECK_DIE_ON_FAILURE
                 chipDie();
 #endif
             }
@@ -147,7 +147,7 @@ ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataModel, const Ac
                 ChipLogError(Test, "Different partial data");
                 // NOTE: die on unit tests only, since partial data size may differ across
                 //       time-dependent data (very rarely because fast code, but still possible)
-#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
+#if CHIP_CONFIG_DATA_MODEL_CHECK_DIE_ON_FAILURE
                 chipDie();
 #endif
             }