Skip to content

Commit

Permalink
Switch up unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@andy31415
andy31415 committed Oct 23, 2024
1 parent e8b5cbe commit ecdca36
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 47 deletions.
27 changes: 2 additions & 25 deletions src/app/codegen-data-model-provider/EmberDataBuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 2 additions & 6 deletions src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2133,9 +2133,7 @@ TEST(TestCodegenModelViaMocks, EmberTestWriteOutOfRepresentableRangeOddIntegerNo
using NullableType = chip::app::DataModel::Nullable<typename NumericType::WorkingType>;
AttributeValueDecoder decoder = test.DecoderFor<NullableType>(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)
Expand All @@ -2151,9 +2149,7 @@ TEST(TestCodegenModelViaMocks, EmberTestWriteOutOfRepresentableRangeOddIntegerNu
using NullableType = chip::app::DataModel::Nullable<typename NumericType::WorkingType>;
AttributeValueDecoder decoder = test.DecoderFor<NullableType>(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)
Expand Down
32 changes: 16 additions & 16 deletions src/app/codegen-data-model-provider/tests/TestEmberDataBuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,8 @@ TEST(TestEmberAttributeBuffer, TestEncodeUnsignedTypes)
EXPECT_TRUE(tester.TryEncode<uint32_t>(0xFFFFFF, { 0xFF, 0xFF, 0xFF }).IsSuccess());

// Out of range
EXPECT_EQ(tester.TryEncode<uint32_t>(0x1000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<uint32_t>(0xFF000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<uint32_t>(0x1000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
EXPECT_EQ(tester.TryEncode<uint32_t>(0xFF000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
}
{
EncodeTester tester(CreateFakeMeta(ZCL_INT24U_ATTRIBUTE_TYPE, true /* nullable */));
Expand All @@ -281,9 +281,9 @@ TEST(TestEmberAttributeBuffer, TestEncodeUnsignedTypes)
EXPECT_TRUE(tester.TryEncode<DataModel::Nullable<uint32_t>>(DataModel::NullNullable, { 0xFF, 0xFF, 0xFF }).IsSuccess());

// Out of range
EXPECT_EQ(tester.TryEncode<uint32_t>(0x1000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<uint32_t>(0x1000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
// cannot encode null equivalent value
EXPECT_EQ(tester.TryEncode<uint32_t>(0xFFFFFF, { 0x56, 0x34, 0x12 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<uint32_t>(0xFFFFFF, { 0x56, 0x34, 0x12 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
}

{
Expand All @@ -295,9 +295,9 @@ TEST(TestEmberAttributeBuffer, TestEncodeUnsignedTypes)
tester.TryEncode<DataModel::Nullable<uint64_t>>(DataModel::NullNullable, { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }).IsSuccess());

// Out of range
EXPECT_EQ(tester.TryEncode<uint64_t>(0x10011001100, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<uint64_t>(0x10011001100, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
// cannot encode null equivalent value
EXPECT_EQ(tester.TryEncode<uint64_t>(0xFFFFFFFFFF, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<uint64_t>(0xFFFFFFFFFF, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
}

// Double-check tests, not as exhaustive, to cover all other unsigned values and get
Expand Down Expand Up @@ -376,9 +376,9 @@ TEST(TestEmberAttributeBuffer, TestEncodeSignedTypes)
EXPECT_TRUE(tester.TryEncode<int32_t>(-1234, { 0x2E, 0xFB, 0xFF }).IsSuccess());

// Out of range
EXPECT_EQ(tester.TryEncode<int32_t>(0x1000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<int32_t>(0x0F000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<int32_t>(-0x1000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<int32_t>(0x1000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
EXPECT_EQ(tester.TryEncode<int32_t>(0x0F000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
EXPECT_EQ(tester.TryEncode<int32_t>(-0x1000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
}
{
EncodeTester tester(CreateFakeMeta(ZCL_INT24S_ATTRIBUTE_TYPE, true /* nullable */));
Expand All @@ -392,14 +392,14 @@ TEST(TestEmberAttributeBuffer, TestEncodeSignedTypes)
EXPECT_TRUE(tester.TryEncode<DataModel::Nullable<uint32_t>>(DataModel::NullNullable, { 0x00, 0x00, 0x80 }).IsSuccess());

// Out of range
EXPECT_EQ(tester.TryEncode<int32_t>(0x1000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<int32_t>(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<int32_t>(-(1 << 24) - 1, { 0x56, 0x34, 0x12 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<int32_t>(-(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<int32_t>(0xFFFFFF, { 0x56, 0x34, 0x12 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<int32_t>(0x800000, { 0x56, 0x34, 0x12 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<int32_t>(0xFFFFFF, { 0x56, 0x34, 0x12 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
EXPECT_EQ(tester.TryEncode<int32_t>(0x800000, { 0x56, 0x34, 0x12 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
}

{
Expand All @@ -421,11 +421,11 @@ TEST(TestEmberAttributeBuffer, TestEncodeSignedTypes)
tester.TryEncode<DataModel::Nullable<uint64_t>>(DataModel::NullNullable, { 0x00, 0x00, 0x00, 0x00, 0x80 }).IsSuccess());

// Out of range
EXPECT_EQ(tester.TryEncode<int64_t>(0x10011001100, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<int64_t>(0x10011001100, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
// cannot encode null equivalent value
EXPECT_EQ(tester.TryEncode<int64_t>(-(1LL << 40) - 1, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<int64_t>(-(1LL << 40) - 1, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
// negative out of range
EXPECT_EQ(tester.TryEncode<int64_t>(-0x10000000000, { 0 }), CHIP_ERROR_INVALID_ARGUMENT);
EXPECT_EQ(tester.TryEncode<int64_t>(-0x10000000000, { 0 }), CHIP_IM_GLOBAL_STATUS(ConstraintError));
}

// Double-check tests, not as exhaustive, to cover all other unsigned values and get
Expand Down

0 comments on commit ecdca36

Please sign in to comment.