From ecdca36dec16109bbe3182c5b0d37873e1d7fb22 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 23 Oct 2024 08:07:15 -0400 Subject: [PATCH] Switch up unit tests --- .../EmberDataBuffer.cpp | 27 ++-------------- .../tests/TestCodegenModelViaMocks.cpp | 8 ++--- .../tests/TestEmberDataBuffer.cpp | 32 +++++++++---------- 3 files changed, 20 insertions(+), 47 deletions(-) diff --git a/src/app/codegen-data-model-provider/EmberDataBuffer.cpp b/src/app/codegen-data-model-provider/EmberDataBuffer.cpp index b9cd576f774e93..68f2541dc63ac8 100644 --- a/src/app/codegen-data-model-provider/EmberDataBuffer.cpp +++ b/src/app/codegen-data-model-provider/EmberDataBuffer.cpp @@ -142,29 +142,6 @@ constexpr SignedDecodeInfo GetSignedDecodeInfo(EmberAfAttributeType type) chipDie(); } -static constexpr bool IsOddIntegerSize(unsigned byteCount) -{ - // All these conditions seem to result in the same code size: - // - (byteCount > 2) && (byteCount != 4) && (byteCount != 8) OR - // - (byteCount == 6) || ((byteCount & 0x1) != 0) - // - // So ended up keeping the "readable" one - return (byteCount == 3) || (byteCount == 5) || (byteCount == 6) || (byteCount == 7); -} - -// This is an odd workaround for legacy. Errors SHOULD be always ConstraintError -// however in practice old ember code returns INVALID_ARGUMENT for odd sized integers -// -// TODO: This should ALWAYS return ConstraintError (and method should not exist ...) -CHIP_ERROR OutOfRangeError(unsigned byteCount) -{ - if (IsOddIntegerSize(byteCount)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - return CHIP_IM_GLOBAL_STATUS(ConstraintError); -} - } // namespace CHIP_ERROR EmberAttributeBuffer::DecodeUnsignedInteger(chip::TLV::TLVReader & reader, EndianWriter & writer) @@ -186,7 +163,7 @@ CHIP_ERROR EmberAttributeBuffer::DecodeUnsignedInteger(chip::TLV::TLVReader & re VerifyOrReturnError(mIsNullable // ::max() on the type is used as the NULL flag ? (value < info.maxValue) : (value <= info.maxValue), - OutOfRangeError(info.byteCount)); + CHIP_IM_GLOBAL_STATUS(ConstraintError)); } writer.EndianPut(value, info.byteCount); @@ -214,7 +191,7 @@ CHIP_ERROR EmberAttributeBuffer::DecodeSignedInteger(chip::TLV::TLVReader & read // - NON-NULLABLE: [minValue, MaxValue] bool valid = (value <= info.maxValue) && (mIsNullable ? (value > info.minValue) : (value >= info.minValue)); - VerifyOrReturnError(valid, OutOfRangeError(info.byteCount)); + VerifyOrReturnError(valid, CHIP_IM_GLOBAL_STATUS(ConstraintError)); } writer.EndianPutSigned(value, info.byteCount); return CHIP_NO_ERROR; diff --git a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp index eccdbe7b91ba71..8ce369b7bd8220 100644 --- a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp @@ -2133,9 +2133,7 @@ TEST(TestCodegenModelViaMocks, EmberTestWriteOutOfRepresentableRangeOddIntegerNo using NullableType = chip::app::DataModel::Nullable; AttributeValueDecoder decoder = test.DecoderFor(0x1223344); - // write should fail: written value is not in range - // NOTE: this matches legacy behaviour, however realistically maybe ConstraintError would be more correct - ASSERT_EQ(model.WriteAttribute(test.GetRequest(), decoder), CHIP_ERROR_INVALID_ARGUMENT); + ASSERT_EQ(model.WriteAttribute(test.GetRequest(), decoder), CHIP_IM_GLOBAL_STATUS(ConstraintError)); } TEST(TestCodegenModelViaMocks, EmberTestWriteOutOfRepresentableRangeOddIntegerNullable) @@ -2151,9 +2149,7 @@ TEST(TestCodegenModelViaMocks, EmberTestWriteOutOfRepresentableRangeOddIntegerNu using NullableType = chip::app::DataModel::Nullable; AttributeValueDecoder decoder = test.DecoderFor(0x1223344); - // write should fail: written value is not in range - // NOTE: this matches legacy behaviour, however realistically maybe ConstraintError would be more correct - ASSERT_EQ(model.WriteAttribute(test.GetRequest(), decoder), CHIP_ERROR_INVALID_ARGUMENT); + ASSERT_EQ(model.WriteAttribute(test.GetRequest(), decoder), CHIP_IM_GLOBAL_STATUS(ConstraintError)); } TEST(TestCodegenModelViaMocksNullValueToNullables, EmberAttributeWriteBasicTypesLowestValue) diff --git a/src/app/codegen-data-model-provider/tests/TestEmberDataBuffer.cpp b/src/app/codegen-data-model-provider/tests/TestEmberDataBuffer.cpp index f2ab3069fa8e7c..7de30e611ddf05 100644 --- a/src/app/codegen-data-model-provider/tests/TestEmberDataBuffer.cpp +++ b/src/app/codegen-data-model-provider/tests/TestEmberDataBuffer.cpp @@ -271,8 +271,8 @@ TEST(TestEmberAttributeBuffer, TestEncodeUnsignedTypes) EXPECT_TRUE(tester.TryEncode(0xFFFFFF, { 0xFF, 0xFF, 0xFF }).IsSuccess()); // Out of range - EXPECT_EQ(tester.TryEncode(0x1000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); - EXPECT_EQ(tester.TryEncode(0xFF000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(0x1000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); + EXPECT_EQ(tester.TryEncode(0xFF000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); } { EncodeTester tester(CreateFakeMeta(ZCL_INT24U_ATTRIBUTE_TYPE, true /* nullable */)); @@ -281,9 +281,9 @@ TEST(TestEmberAttributeBuffer, TestEncodeUnsignedTypes) EXPECT_TRUE(tester.TryEncode>(DataModel::NullNullable, { 0xFF, 0xFF, 0xFF }).IsSuccess()); // Out of range - EXPECT_EQ(tester.TryEncode(0x1000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(0x1000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); // cannot encode null equivalent value - EXPECT_EQ(tester.TryEncode(0xFFFFFF, { 0x56, 0x34, 0x12 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(0xFFFFFF, { 0x56, 0x34, 0x12 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); } { @@ -295,9 +295,9 @@ TEST(TestEmberAttributeBuffer, TestEncodeUnsignedTypes) tester.TryEncode>(DataModel::NullNullable, { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }).IsSuccess()); // Out of range - EXPECT_EQ(tester.TryEncode(0x10011001100, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(0x10011001100, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); // cannot encode null equivalent value - EXPECT_EQ(tester.TryEncode(0xFFFFFFFFFF, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(0xFFFFFFFFFF, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); } // Double-check tests, not as exhaustive, to cover all other unsigned values and get @@ -376,9 +376,9 @@ TEST(TestEmberAttributeBuffer, TestEncodeSignedTypes) EXPECT_TRUE(tester.TryEncode(-1234, { 0x2E, 0xFB, 0xFF }).IsSuccess()); // Out of range - EXPECT_EQ(tester.TryEncode(0x1000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); - EXPECT_EQ(tester.TryEncode(0x0F000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); - EXPECT_EQ(tester.TryEncode(-0x1000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(0x1000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); + EXPECT_EQ(tester.TryEncode(0x0F000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); + EXPECT_EQ(tester.TryEncode(-0x1000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); } { EncodeTester tester(CreateFakeMeta(ZCL_INT24S_ATTRIBUTE_TYPE, true /* nullable */)); @@ -392,14 +392,14 @@ TEST(TestEmberAttributeBuffer, TestEncodeSignedTypes) EXPECT_TRUE(tester.TryEncode>(DataModel::NullNullable, { 0x00, 0x00, 0x80 }).IsSuccess()); // Out of range - EXPECT_EQ(tester.TryEncode(0x1000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(0x1000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); // cannot encode null equivalent value - this is the minimum negative value // for 24-bit - EXPECT_EQ(tester.TryEncode(-(1 << 24) - 1, { 0x56, 0x34, 0x12 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(-(1 << 24) - 1, { 0x56, 0x34, 0x12 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); // Out of range for signed - these are unsigned values that are larger - EXPECT_EQ(tester.TryEncode(0xFFFFFF, { 0x56, 0x34, 0x12 }), CHIP_ERROR_INVALID_ARGUMENT); - EXPECT_EQ(tester.TryEncode(0x800000, { 0x56, 0x34, 0x12 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(0xFFFFFF, { 0x56, 0x34, 0x12 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); + EXPECT_EQ(tester.TryEncode(0x800000, { 0x56, 0x34, 0x12 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); } { @@ -421,11 +421,11 @@ TEST(TestEmberAttributeBuffer, TestEncodeSignedTypes) tester.TryEncode>(DataModel::NullNullable, { 0x00, 0x00, 0x00, 0x00, 0x80 }).IsSuccess()); // Out of range - EXPECT_EQ(tester.TryEncode(0x10011001100, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(0x10011001100, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); // cannot encode null equivalent value - EXPECT_EQ(tester.TryEncode(-(1LL << 40) - 1, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(-(1LL << 40) - 1, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); // negative out of range - EXPECT_EQ(tester.TryEncode(-0x10000000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(tester.TryEncode(-0x10000000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError)); } // Double-check tests, not as exhaustive, to cover all other unsigned values and get