From ce4845f8955109ecf29dc18b66c3c68996a497fe Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 30 Aug 2024 12:53:39 -0400 Subject: [PATCH] Replace usage of CONFIG_BUILD_FOR_HOST_UNIT_TEST for IM/DM validation checker (#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 * make the flag name singular --------- Co-authored-by: Andrei Litvin Co-authored-by: Boris Zbarsky --- .github/workflows/build.yaml | 10 +++++----- .github/workflows/unit_integration_test.yaml | 2 +- scripts/build/builders/host.py | 1 + src/app/BUILD.gn | 1 + src/app/common_flags.gni | 10 +++++++--- src/app/reporting/Read-Checked.cpp | 8 ++++---- 6 files changed, 19 insertions(+), 13 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 3e4f9d57056f63..be259e232e5022 100644 --- 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 index 5716df1a5d33af..040e50f1535079 100644 --- 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 index 8c03ac7b133501..3c4bb6b672e172 100644 --- 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 index 53dd3876c12410..8327bef57008aa 100644 --- 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 index 6a194058aff59a..953afc23a20650 100644 --- 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 index 2bb969aac094d2..4336668f5764ea 100644 --- 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 }