Skip to content

Commit

Permalink
Add a pigweed-string-format adapter for CHIP_ERROR since that makes u…
Browse files Browse the repository at this point in the history
…nit test debugability a LOT better (project-chip#33505)

* Define StringBuilderAdapters

* Make use of string builder adapters, switch gtest to pw unittest framework

* Restyle

* Give explicit examples of how string adapters help

* Place implementation of ToString in a cpp file

* Add missing file

---------

Co-authored-by: Andrei Litvin <[email protected]>
  • Loading branch information
andy31415 and andreilitvin authored May 20, 2024
1 parent 7a470f5 commit c021fad
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 5 deletions.
13 changes: 13 additions & 0 deletions src/lib/core/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import("//build_overrides/build.gni")
import("//build_overrides/chip.gni")
import("//build_overrides/nlio.gni")
import("//build_overrides/pigweed.gni")

import("${chip_root}/build/chip/buildconfig_header.gni")
import("${chip_root}/build/chip/tests.gni")
Expand Down Expand Up @@ -106,6 +107,18 @@ source_set("error") {
]
}

source_set("string-builder-adapters") {
sources = [
"StringBuilderAdapters.cpp",
"StringBuilderAdapters.h",
]

public_deps = [
":error",
"$dir_pw_string",
]
}

source_set("encoding") {
sources = [ "CHIPEncoding.h" ]
public_deps = [ "${nlio_root}:nlio" ]
Expand Down
31 changes: 31 additions & 0 deletions src/lib/core/StringBuilderAdapters.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <lib/core/StringBuilderAdapters.h>

namespace pw {

template <>
StatusWithSize ToString<CHIP_ERROR>(const CHIP_ERROR & err, pw::span<char> buffer)
{
if (CHIP_ERROR::IsSuccess(err))
{
// source location probably does not matter
return pw::string::Format(buffer, "CHIP_NO_ERROR");
}
return pw::string::Format(buffer, "CHIP_ERROR:<%" CHIP_ERROR_FORMAT ">", err.Format());
}

} // namespace pw
53 changes: 53 additions & 0 deletions src/lib/core/StringBuilderAdapters.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#pragma once

/// This header includes pigweed stringbuilder adaptations for various chip types.
/// You can see https://pigweed.dev/pw_string/guide.html as a reference.
///
/// In particular, pigweed code generally looks like:
///
/// pw::StringBuffer<42> sb;
/// sb << "Here is a value: ";
/// sb << value;
///
/// Where specific formatters exist for "value". In particular these are used when
/// reporting unit test assertions such as ASSERT_EQ/EXPECT_EQ so if you write code
/// like:
///
/// ASSERT_EQ(SomeCall(), CHIP_NO_ERROR);
///
/// On failure without adapters, the objects are reported as "24-byte object at 0x....."
/// which is not as helpful as a full CHIP_ERROR formatted output.
///
/// Example output WITHOUT the adapters
/// Expected: .... == CHIP_ERROR(0, "src/setup_payload/tests/TestAdditionalDataPayload.cpp", 234)
/// Actual: <24-byte object at 0x7ffe39510b80> == <24-byte object at 0x7ffe39510ba0>
///
/// Example output WITH the adapters:
/// Expected: .... == CHIP_ERROR(0, "src/setup_payload/tests/TestAdditionalDataPayload.cpp", 234)
/// Actual: CHIP_ERROR:<src/lib/core/TLVReader.cpp:889: Error 0x00000022> == CHIP_NO_ERROR

#include <pw_string/string_builder.h>

#include <lib/core/CHIPError.h>

namespace pw {

template <>
StatusWithSize ToString<CHIP_ERROR>(const CHIP_ERROR & err, pw::span<char> buffer);

} // namespace pw
1 change: 1 addition & 0 deletions src/setup_payload/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ chip_test_suite("tests") {
cflags = [ "-Wconversion" ]

public_deps = [
"${chip_root}/src/lib/core:string-builder-adapters",
"${chip_root}/src/platform",
"${chip_root}/src/setup_payload",
]
Expand Down
1 change: 1 addition & 0 deletions src/setup_payload/tests/TestAdditionalDataPayload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#include <gtest/gtest.h>

#include <lib/core/StringBuilderAdapters.h>
#include <lib/support/BytesToHex.h>
#include <lib/support/CHIPMem.h>
#include <lib/support/CHIPMemString.h>
Expand Down
6 changes: 3 additions & 3 deletions src/setup_payload/tests/TestManualCode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@
*
*/

#include <gtest/gtest.h>
#include <pw_unit_test/framework.h>
#include <stdio.h>

#include <lib/core/StringBuilderAdapters.h>
#include <lib/support/verhoeff/Verhoeff.h>
#include <setup_payload/ManualSetupPayloadGenerator.h>
#include <setup_payload/ManualSetupPayloadParser.h>
#include <setup_payload/SetupPayload.h>

#include <lib/support/verhoeff/Verhoeff.h>

#include <algorithm>
#include <math.h>
#include <string>
Expand Down
4 changes: 3 additions & 1 deletion src/setup_payload/tests/TestQRCode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@

#include <nlbyteorder.h>

#include <gtest/gtest.h>
#include <pw_unit_test/framework.h>

#include <lib/core/StringBuilderAdapters.h>
#include <lib/support/Span.h>

using namespace chip;
Expand Down
4 changes: 3 additions & 1 deletion src/setup_payload/tests/TestQRCodeTLV.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@
*/
#include "TestHelpers.h"

#include <gtest/gtest.h>
#include <pw_unit_test/framework.h>

#include <nlbyteorder.h>

#include <lib/core/StringBuilderAdapters.h>
#include <lib/support/ScopedBuffer.h>

using namespace chip;
Expand Down

0 comments on commit c021fad

Please sign in to comment.