From df2976dfd895ae0f646260373bc8c35c171ae8a4 Mon Sep 17 00:00:00 2001
From: Xu <xzhseh@gmail.com>
Date: Fri, 11 Aug 2023 22:38:51 +0800
Subject: [PATCH] fix(test): ensure compatibility std::uniform_int_distribution
 for binary data generation & improve overall format (#587)

fix: ensure compatibility std::uniform_int_distribution for binary data generation & improve overall format
---
 test/buffer/buffer_pool_manager_test.cpp | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/test/buffer/buffer_pool_manager_test.cpp b/test/buffer/buffer_pool_manager_test.cpp
index 2d080ed1e..440c85cb9 100644
--- a/test/buffer/buffer_pool_manager_test.cpp
+++ b/test/buffer/buffer_pool_manager_test.cpp
@@ -13,6 +13,7 @@
 #include "buffer/buffer_pool_manager.h"
 
 #include <cstdio>
+#include <limits>
 #include <random>
 #include <string>
 
@@ -29,7 +30,12 @@ TEST(BufferPoolManagerTest, DISABLED_BinaryDataTest) {
 
   std::random_device r;
   std::default_random_engine rng(r());
-  std::uniform_int_distribution<char> uniform_dist(0);
+
+  constexpr int lower_bound = static_cast<int>(std::numeric_limits<char>::min());
+  constexpr int upper_bound = static_cast<int>(std::numeric_limits<char>::max());
+  // No matter if `char` is signed or unsigned by default, this constraint must be met
+  static_assert(upper_bound - lower_bound == 255);
+  std::uniform_int_distribution<int> uniform_dist(lower_bound, upper_bound);
 
   auto *disk_manager = new DiskManager(db_name);
   auto *bpm = new BufferPoolManager(buffer_pool_size, disk_manager, k);
@@ -44,7 +50,7 @@ TEST(BufferPoolManagerTest, DISABLED_BinaryDataTest) {
   char random_binary_data[BUSTUB_PAGE_SIZE];
   // Generate random binary data
   for (char &i : random_binary_data) {
-    i = uniform_dist(rng);
+    i = static_cast<char>(uniform_dist(rng));
   }
 
   // Insert terminal characters both in the middle and at end
@@ -65,17 +71,20 @@ TEST(BufferPoolManagerTest, DISABLED_BinaryDataTest) {
     EXPECT_EQ(nullptr, bpm->NewPage(&page_id_temp));
   }
 
-  // Scenario: After unpinning pages {0, 1, 2, 3, 4} we should be able to create 5 new pages
+  // Scenario: After unpinning pages {0, 1, 2, 3, 4}, we should be able to create 5 new pages
   for (int i = 0; i < 5; ++i) {
     EXPECT_EQ(true, bpm->UnpinPage(i, true));
     bpm->FlushPage(i);
   }
   for (int i = 0; i < 5; ++i) {
     EXPECT_NE(nullptr, bpm->NewPage(&page_id_temp));
+    // Unpin the page here to allow future fetching
     bpm->UnpinPage(page_id_temp, false);
   }
+
   // Scenario: We should be able to fetch the data we wrote a while ago.
   page0 = bpm->FetchPage(0);
+  ASSERT_NE(nullptr, page0);
   EXPECT_EQ(0, memcmp(page0->GetData(), random_binary_data, BUSTUB_PAGE_SIZE));
   EXPECT_EQ(true, bpm->UnpinPage(0, true));
 
@@ -128,10 +137,11 @@ TEST(BufferPoolManagerTest, DISABLED_SampleTest) {
 
   // Scenario: We should be able to fetch the data we wrote a while ago.
   page0 = bpm->FetchPage(0);
+  ASSERT_NE(nullptr, page0);
   EXPECT_EQ(0, strcmp(page0->GetData(), "Hello"));
 
   // Scenario: If we unpin page 0 and then make a new page, all the buffer pages should
-  // now be pinned. Fetching page 0 should fail.
+  // now be pinned. Fetching page 0 again should fail.
   EXPECT_EQ(true, bpm->UnpinPage(0, true));
   EXPECT_NE(nullptr, bpm->NewPage(&page_id_temp));
   EXPECT_EQ(nullptr, bpm->FetchPage(0));