pw_kvs: Expand tests for error handling
- Add error injection features to InMemoryFakeFlash. Reads or Writes can
be programmed to fail.
- Introduce key_value_store_error_handling_test.cc, which focuses on
corruption and read/write failure cases.
- Add functions for creating binary KVS entries at compile time.
Change-Id: Ie52ba5eb13eb60244835ef43314282beacc3a659
diff --git a/.gitignore b/.gitignore
index 9c7800a..a2708bc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -11,6 +11,7 @@
.vscode
.clangd/
*.swp
+*.swo
# Python
python*-env/
diff --git a/pw_kvs/BUILD b/pw_kvs/BUILD
index 01b63ea..4083387 100644
--- a/pw_kvs/BUILD
+++ b/pw_kvs/BUILD
@@ -123,7 +123,19 @@
":crc16",
":pw_kvs",
":test_utils",
- "//pw_checksum",
+ "//pw_log",
+ ],
+)
+
+pw_cc_test(
+ name = "key_value_store_error_handling_test",
+ srcs = [
+ "key_value_store_error_handling_test.cc",
+ ],
+ deps = [
+ ":crc16",
+ ":pw_kvs",
+ ":test_utils",
"//pw_log",
],
)
diff --git a/pw_kvs/BUILD.gn b/pw_kvs/BUILD.gn
index 5f899b3..3256401 100644
--- a/pw_kvs/BUILD.gn
+++ b/pw_kvs/BUILD.gn
@@ -55,6 +55,7 @@
friend = [
":entry_test",
":key_value_store_test",
+ ":key_value_store_error_handling_test",
":key_value_store_map_test",
]
}
@@ -101,6 +102,7 @@
":checksum_test",
":entry_test",
":key_value_store_test",
+ ":key_value_store_error_handling_test",
":key_value_store_fuzz_test",
":key_value_store_map_test",
]
@@ -150,6 +152,18 @@
]
}
+pw_test("key_value_store_error_handling_test") {
+ deps = [
+ ":crc16",
+ ":pw_kvs",
+ ":test_utils",
+ dir_pw_log,
+ ]
+ sources = [
+ "key_value_store_error_handling_test.cc",
+ ]
+}
+
pw_test("key_value_store_fuzz_test") {
deps = [
":crc16",
diff --git a/pw_kvs/in_memory_fake_flash.cc b/pw_kvs/in_memory_fake_flash.cc
index 70f3890..0b91965 100644
--- a/pw_kvs/in_memory_fake_flash.cc
+++ b/pw_kvs/in_memory_fake_flash.cc
@@ -18,6 +18,42 @@
namespace pw::kvs {
+FlashError::Result FlashError::Check(span<FlashError> errors,
+ FlashMemory::Address address,
+ size_t size) {
+ for (auto& error : errors) {
+ if (Result result = error.Check(address, size); !result.status.ok()) {
+ return result;
+ }
+ }
+
+ return {Status::OK, true};
+}
+
+FlashError::Result FlashError::Check(FlashMemory::Address start_address,
+ size_t size) {
+ // Check if the event overlaps with this address range.
+ if (begin_ != kAnyAddress &&
+ (start_address >= end_ || (start_address + size) < begin_)) {
+ return {Status::OK, true};
+ }
+
+ if (delay_ > 0u) {
+ delay_ -= 1;
+ return {Status::OK, true};
+ }
+
+ if (remaining_ == 0u) {
+ return {Status::OK, true};
+ }
+
+ if (remaining_ != kAlways) {
+ remaining_ -= 1;
+ }
+
+ return {status_, mode_ != kAbort};
+}
+
Status InMemoryFakeFlash::Erase(Address address, size_t num_sectors) {
if (address % sector_size_bytes() != 0) {
PW_LOG_ERROR(
@@ -45,8 +81,14 @@
if (address + output.size() >= sector_count() * size_bytes()) {
return StatusWithSize(Status::OUT_OF_RANGE);
}
- std::memcpy(output.data(), &buffer_[address], output.size());
- return StatusWithSize(output.size());
+
+ // Check for injected read errors
+ auto [status, finish_operation] =
+ FlashError::Check(read_errors_, address, output.size());
+ if (finish_operation) {
+ std::memcpy(output.data(), &buffer_[address], output.size());
+ }
+ return StatusWithSize(status, output.size());
}
StatusWithSize InMemoryFakeFlash::Write(Address address,
@@ -84,7 +126,14 @@
return StatusWithSize(Status::UNKNOWN);
}
}
- std::memcpy(&buffer_[address], data.data(), data.size());
- return StatusWithSize(data.size());
+
+ // Check for any injected write errors
+ auto [status, finish_operation] =
+ FlashError::Check(write_errors_, address, data.size());
+ if (finish_operation) {
+ std::memcpy(&buffer_[address], data.data(), data.size());
+ }
+ return StatusWithSize(status, data.size());
}
+
} // namespace pw::kvs
diff --git a/pw_kvs/key_value_store_error_handling_test.cc b/pw_kvs/key_value_store_error_handling_test.cc
new file mode 100644
index 0000000..cf4fa38
--- /dev/null
+++ b/pw_kvs/key_value_store_error_handling_test.cc
@@ -0,0 +1,169 @@
+// Copyright 2020 The Pigweed 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
+//
+// https://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 <string_view>
+
+#include "gtest/gtest.h"
+#include "pw_kvs/crc16_checksum.h"
+#include "pw_kvs/in_memory_fake_flash.h"
+#include "pw_kvs/internal/hash.h"
+#include "pw_kvs/key_value_store.h"
+#include "pw_kvs_private/byte_utils.h"
+
+namespace pw::kvs {
+namespace {
+
+using std::byte;
+using std::string_view;
+
+constexpr size_t kMaxEntries = 256;
+constexpr size_t kMaxUsableSectors = 256;
+
+constexpr uint32_t SimpleChecksum(span<const byte> data, uint32_t state = 0) {
+ for (byte b : data) {
+ state += uint32_t(b);
+ }
+ return state;
+}
+
+class SimpleChecksumAlgorithm final : public ChecksumAlgorithm {
+ public:
+ SimpleChecksumAlgorithm()
+ : ChecksumAlgorithm(as_bytes(span(&checksum_, 1))) {}
+
+ void Reset() override { checksum_ = 0; }
+
+ void Update(span<const byte> data) override {
+ checksum_ = SimpleChecksum(data, checksum_);
+ }
+
+ private:
+ uint32_t checksum_;
+} checksum;
+
+// Returns a buffer containing the necessary padding for an entry.
+template <size_t kAlignmentBytes, size_t kKeyLength, size_t kValueSize>
+constexpr auto EntryPadding() {
+ constexpr size_t content =
+ sizeof(internal::EntryHeader) + kKeyLength + kValueSize;
+ return std::array<byte, Padding(content, kAlignmentBytes)>{};
+}
+
+// Creates a buffer containing a valid entry at compile time.
+template <size_t kAlignmentBytes = sizeof(internal::EntryHeader),
+ size_t kKeyLengthWithNull,
+ size_t kValueSize>
+constexpr auto MakeValidEntry(uint32_t magic,
+ uint32_t id,
+ const char (&key)[kKeyLengthWithNull],
+ const std::array<byte, kValueSize>& value) {
+ constexpr size_t kKeyLength = kKeyLengthWithNull - 1;
+
+ auto data = AsBytes(magic,
+ uint32_t(0),
+ uint8_t(kAlignmentBytes / 16 - 1),
+ uint8_t(kKeyLength),
+ uint16_t(kValueSize),
+ id,
+ ByteStr(key),
+ span(value),
+ EntryPadding<kAlignmentBytes, kKeyLength, kValueSize>());
+
+ // Calculate the checksum
+ uint32_t checksum = SimpleChecksum(data);
+ for (size_t i = 0; i < sizeof(checksum); ++i) {
+ data[4 + i] = byte(checksum & 0xff);
+ checksum >>= 8;
+ }
+
+ return data;
+}
+
+constexpr uint32_t kMagic = 0xc001beef;
+constexpr EntryFormat kFormat{.magic = kMagic, .checksum = &checksum};
+
+constexpr auto kEntry1 = MakeValidEntry(kMagic, 1, "key1", ByteStr("value1"));
+constexpr auto kEntry2 = MakeValidEntry(kMagic, 3, "k2", ByteStr("value2"));
+
+class KvsErrorHandling : public ::testing::Test {
+ protected:
+ KvsErrorHandling()
+ : flash_(internal::Entry::kMinAlignmentBytes),
+ partition_(&flash_),
+ kvs_(&partition_, kFormat) {}
+
+ void InitFlashTo(span<const byte> contents) {
+ partition_.Erase();
+ std::memcpy(flash_.buffer().data(), contents.data(), contents.size());
+ }
+
+ FakeFlashBuffer<512, 4> flash_;
+ FlashPartition partition_;
+ KeyValueStoreBuffer<kMaxEntries, kMaxUsableSectors> kvs_;
+};
+
+TEST_F(KvsErrorHandling, Init_Ok) {
+ InitFlashTo(AsBytes(kEntry1, kEntry2));
+
+ EXPECT_EQ(Status::OK, kvs_.Init());
+ byte buffer[64];
+ EXPECT_EQ(Status::OK, kvs_.Get("key1", buffer).status());
+ EXPECT_EQ(Status::OK, kvs_.Get("k2", buffer).status());
+}
+
+TEST_F(KvsErrorHandling, Init_DuplicateEntries_ReturnsDataLossButReadsEntry) {
+ InitFlashTo(AsBytes(kEntry1, kEntry1));
+
+ EXPECT_EQ(Status::DATA_LOSS, kvs_.Init());
+ byte buffer[64];
+ EXPECT_EQ(Status::OK, kvs_.Get("key1", buffer).status());
+ EXPECT_EQ(Status::NOT_FOUND, kvs_.Get("k2", buffer).status());
+}
+
+TEST_F(KvsErrorHandling, Init_CorruptEntry_FindsSubsequentValidEntry) {
+ // Corrupt each byte in the first entry once.
+ for (size_t i = 0; i < kEntry1.size(); ++i) {
+ InitFlashTo(AsBytes(kEntry1, kEntry2));
+ flash_.buffer()[i] = byte(int(flash_.buffer()[i]) + 1);
+
+ ASSERT_EQ(Status::DATA_LOSS, kvs_.Init());
+ byte buffer[64];
+ ASSERT_EQ(Status::NOT_FOUND, kvs_.Get("key1", buffer).status());
+ ASSERT_EQ(Status::OK, kvs_.Get("k2", buffer).status());
+ }
+}
+
+TEST_F(KvsErrorHandling, Init_ReadError_IsNotInitialized) {
+ InitFlashTo(AsBytes(kEntry1, kEntry2));
+
+ flash_.InjectReadError(
+ FlashError::InRange(Status::UNAUTHENTICATED, kEntry1.size()));
+
+ EXPECT_EQ(Status::UNKNOWN, kvs_.Init());
+ EXPECT_FALSE(kvs_.initialized());
+}
+
+TEST_F(KvsErrorHandling, Put_WriteFailure_EntryNotAdded) {
+ ASSERT_EQ(Status::OK, kvs_.Init());
+ flash_.InjectWriteError(FlashError::Unconditional(Status::UNAVAILABLE));
+
+ EXPECT_EQ(Status::UNAVAILABLE, kvs_.Put("key1", ByteStr("value1")));
+
+ byte buffer[64];
+ EXPECT_EQ(Status::NOT_FOUND, kvs_.Get("key1", buffer).status());
+ ASSERT_TRUE(kvs_.empty());
+}
+
+} // namespace
+} // namespace pw::kvs
diff --git a/pw_kvs/public/pw_kvs/in_memory_fake_flash.h b/pw_kvs/public/pw_kvs/in_memory_fake_flash.h
index 761e57c..2bb385d 100644
--- a/pw_kvs/public/pw_kvs/in_memory_fake_flash.h
+++ b/pw_kvs/public/pw_kvs/in_memory_fake_flash.h
@@ -18,16 +18,86 @@
#include <cstddef>
#include <cstring>
+#include "pw_containers/vector.h"
#include "pw_kvs/flash_memory.h"
#include "pw_span/span.h"
#include "pw_status/status.h"
namespace pw::kvs {
+class FlashError {
+ public:
+ enum Mode {
+ kAbort,
+ kContinueButReportError,
+ };
+
+ static constexpr FlashMemory::Address kAnyAddress = FlashMemory::Address(-1);
+ static constexpr size_t kAlways = size_t(-1);
+
+ // Creates a FlashError that always triggers on the next operation.
+ static constexpr FlashError Unconditional(Status status,
+ Mode mode = kAbort,
+ size_t times = kAlways,
+ size_t delay = 0) {
+ return FlashError(status, kAnyAddress, 0, mode, times, delay);
+ }
+
+ // Creates a FlashError that triggers for particular addresses.
+ static constexpr FlashError InRange(Status status,
+ FlashMemory::Address address,
+ size_t size = 1,
+ Mode mode = kAbort,
+ size_t times = kAlways,
+ size_t delay = 0) {
+ return FlashError(status, address, size, mode, times, delay);
+ }
+
+ struct Result {
+ Status status; // what to return from the operation
+ bool finish_operation; // whether to complete the operation
+ };
+
+ // Determines if this FlashError applies to the operation.
+ Result Check(FlashMemory::Address start_address, size_t size);
+
+ // Determines if any of a series of FlashErrors applies to the operation.
+ static Result Check(span<FlashError> errors,
+ FlashMemory::Address address,
+ size_t size);
+
+ private:
+ constexpr FlashError(Status status,
+ FlashMemory::Address address,
+ size_t size, // not used if address is kAnyAddress
+ Mode mode,
+ size_t times,
+ size_t delay)
+ : status_(status),
+ begin_(address),
+ end_(address + size), // not used if address is kAnyAddress
+ mode_(mode),
+ delay_(delay),
+ remaining_(times) {}
+
+ const Status status_;
+
+ const FlashMemory::Address begin_;
+ const FlashMemory::Address end_; // exclusive
+
+ const Mode mode_;
+
+ size_t delay_;
+ size_t remaining_;
+};
+
// This uses a buffer to mimic the behaviour of flash (requires erase before
// write, checks alignments, and is addressed in sectors). The underlying buffer
// is not initialized.
class InMemoryFakeFlash : public FlashMemory {
+ private:
+ static Vector<FlashError, 0> no_errors_;
+
public:
// Default to 8-bit alignment.
static constexpr size_t kDefaultAlignmentBytes = 1;
@@ -37,9 +107,13 @@
InMemoryFakeFlash(span<std::byte> buffer,
size_t sector_size,
size_t sector_count,
- size_t alignment_bytes = kDefaultAlignmentBytes)
+ size_t alignment_bytes = kDefaultAlignmentBytes,
+ Vector<FlashError>& read_errors = no_errors_,
+ Vector<FlashError>& write_errors = no_errors_)
: FlashMemory(sector_size, sector_count, alignment_bytes),
- buffer_(buffer) {}
+ buffer_(buffer),
+ read_errors_(read_errors),
+ write_errors_(write_errors) {}
// The fake flash is always enabled.
Status Enable() override { return Status::OK; }
@@ -57,18 +131,38 @@
// Writes bytes to flash.
StatusWithSize Write(Address address, span<const std::byte> data) override;
+ // Testing API
+
// Access the underlying buffer for testing purposes. Not part of the
// FlashMemory API.
span<std::byte> buffer() const { return buffer_; }
+ bool InjectReadError(const FlashError& error) {
+ if (read_errors_.full()) {
+ return false;
+ }
+ read_errors_.push_back(error);
+ return true;
+ }
+
+ bool InjectWriteError(const FlashError& error) {
+ if (write_errors_.full()) {
+ return false;
+ }
+ write_errors_.push_back(error);
+ return true;
+ }
+
private:
const span<std::byte> buffer_;
+ Vector<FlashError>& read_errors_;
+ Vector<FlashError>& write_errors_;
};
// Creates an InMemoryFakeFlash backed by a std::array. The array is initialized
// to the erased value. A byte array to which to initialize the memory may be
// provided.
-template <size_t kSectorSize, size_t kSectorCount>
+template <size_t kSectorSize, size_t kSectorCount, size_t kInjectedErrors = 8>
class FakeFlashBuffer : public InMemoryFakeFlash {
public:
// Creates a flash memory with no data written.
@@ -78,7 +172,12 @@
// Creates a flash memory initialized to the provided contents.
FakeFlashBuffer(span<const std::byte> contents,
size_t alignment_bytes = kDefaultAlignmentBytes)
- : InMemoryFakeFlash(buffer_, kSectorSize, kSectorCount, alignment_bytes) {
+ : InMemoryFakeFlash(buffer_,
+ kSectorSize,
+ kSectorCount,
+ alignment_bytes,
+ read_errors_,
+ write_errors_) {
std::memset(buffer_.data(), int(kErasedValue), buffer_.size());
std::memcpy(buffer_.data(),
contents.data(),
@@ -87,6 +186,8 @@
private:
std::array<std::byte, kSectorCount * kSectorSize> buffer_;
+ Vector<FlashError, kInjectedErrors> read_errors_;
+ Vector<FlashError, kInjectedErrors> write_errors_;
};
} // namespace pw::kvs
diff --git a/pw_kvs/pw_kvs_private/byte_utils.h b/pw_kvs/pw_kvs_private/byte_utils.h
index 8eeafbc..8ed74ba 100644
--- a/pw_kvs/pw_kvs_private/byte_utils.h
+++ b/pw_kvs/pw_kvs_private/byte_utils.h
@@ -43,10 +43,20 @@
}
}
-// Converts a series of integers to a std::byte array at compile time.
+template <typename T>
+constexpr size_t SizeOfBytes(const T& arg) {
+ if constexpr (std::is_integral_v<T>) {
+ return sizeof(arg);
+ } else {
+ return arg.size();
+ }
+}
+
+// Converts a series of integers or byte arrays to a std::byte array at compile
+// time.
template <typename... Args>
constexpr auto AsBytes(Args... args) {
- std::array<std::byte, (sizeof(args) + ...)> bytes{};
+ std::array<std::byte, (SizeOfBytes(args) + ...)> bytes{};
auto iterator = bytes.begin();
CopyBytes(iterator, args...);
