Skip to content

Commit

Permalink
Replace usage of CONFIG_BUILD_FOR_HOST_UNIT_TEST for IM/DM validation…
Browse files Browse the repository at this point in the history
… 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 <bzbarsky@apple.com>

* make the flag name singular

---------

Co-authored-by: Andrei Litvin <andreilitvin@google.com>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
  • Loading branch information
3 people authored Aug 30, 2024
1 parent a79c1a8 commit de9d906
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 13 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/unit_integration_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions scripts/build/builders/host.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
10 changes: 7 additions & 3 deletions src/app/common_flags.gni
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
8 changes: 4 additions & 4 deletions src/app/reporting/Read-Checked.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down

0 comments on commit de9d906

Please sign in to comment.