wgtmac commented on code in PR #152:
URL: https://github.com/apache/iceberg-cpp/pull/152#discussion_r2253066793


##########
test/in_memory_catalog_test.cc:
##########
@@ -115,6 +114,50 @@ TEST_F(InMemoryCatalogTest, RegisterTable) {
   ASSERT_EQ(table.value()->location(), "s3://bucket/test/location");
 }
 
+TEST_F(InMemoryCatalogTest, RefreshTable) {
+  TableIdentifier tableIdent{.ns = {}, .name = "t1"};
+  std::shared_ptr<MockInMemoryCatalog> mock_catalog =
+      std::make_shared<MockInMemoryCatalog>(
+          "mock_catalog", file_io_, "/tmp/warehouse/",
+          std::unordered_map<std::string, std::string>());
+  auto table_location = GenerateTestTableLocation(tableIdent.name);
+  auto buildTable = [this, &tableIdent, &mock_catalog, &table_location](
+                        int64_t snapshot_id, int64_t version) -> 
std::unique_ptr<Table> {
+    std::unique_ptr<TableMetadata> metadata;
+
+    ReadTableMetadata("TableMetadataV2Valid.json", &metadata);

Review Comment:
   I would recommend to do the following:
   
   ```cpp
   class MockCatalog : public Catalog {
    public:
     MockCatalog() = default;
     ~MockCatalog() override = default;
   
     MOCK_METHOD(std::string_view, name, (), (const, override));
     MOCK_METHOD(Status, CreateNamespace,
                 (const Namespace&, (const std::unordered_map<std::string, 
std::string>&)),
                 (override));
     MOCK_METHOD((Result<std::vector<Namespace>>), ListNamespaces, (const 
Namespace&),
                 (const, override));
     MOCK_METHOD((Result<std::unordered_map<std::string, std::string>>),
                 GetNamespaceProperties, (const Namespace&), (const, override));
     MOCK_METHOD(Status, UpdateNamespaceProperties,
                 (const Namespace&, (const std::unordered_map<std::string, 
std::string>&),
                  (const std::unordered_set<std::string>&)),
                 (override));
     MOCK_METHOD(Status, DropNamespace, (const Namespace&), (override));
     MOCK_METHOD(Result<bool>, NamespaceExists, (const Namespace&), (const, 
override));
     MOCK_METHOD((Result<std::vector<TableIdentifier>>), ListTables, (const 
Namespace&),
                 (const, override));
     MOCK_METHOD((Result<std::unique_ptr<Table>>), LoadTable, (const 
TableIdentifier&),
                 (override));
     MOCK_METHOD(Status, DropTable, (const TableIdentifier&, bool), (override));
     MOCK_METHOD(Result<bool>, TableExists, (const TableIdentifier&), (const, 
override));
     MOCK_METHOD((Result<std::shared_ptr<Table>>), RegisterTable,
                 (const TableIdentifier&, const std::string&), (override));
     MOCK_METHOD((std::unique_ptr<Catalog::TableBuilder>), BuildTable,
                 (const TableIdentifier&, const Schema&), (const, override));
     MOCK_METHOD((Result<std::unique_ptr<Table>>), CreateTable,
                 (const TableIdentifier&, const Schema&, const PartitionSpec&,
                  const std::string&, (const std::unordered_map<std::string, 
std::string>&)),
                 (override));
     MOCK_METHOD((Result<std::unique_ptr<Table>>), UpdateTable,
                 (const TableIdentifier&,
                  (const std::vector<std::unique_ptr<UpdateRequirement>>&),
                  (const std::vector<std::unique_ptr<MetadataUpdate>>&)),
                 (override));
     MOCK_METHOD((Result<std::shared_ptr<Transaction>>), StageCreateTable,
                 (const TableIdentifier&, const Schema&, const PartitionSpec&,
                  const std::string&, (const std::unordered_map<std::string, 
std::string>&)),
                 (override));
   };
   
   TEST(CatalogTest, MockCatalog) {
     TableIdentifier table_ident{.ns = {}, .name = "t1"};
     auto schema = std::make_shared<Schema>(
         std::vector<SchemaField>{SchemaField::MakeRequired(1, "x", int64())},
         /*schema_id=*/1);
   
     std::shared_ptr<FileIO> io;
   
     auto catalog = std::make_shared<MockCatalog>();
     EXPECT_CALL(*catalog, name()).WillOnce(::testing::Return("test_catalog"));
     EXPECT_EQ(catalog->name(), "test_catalog");
   
     // Mock 1st call to LoadTable
     EXPECT_CALL(*catalog, LoadTable(::testing::_))
         .WillOnce(::testing::Return(
             std::make_unique<Table>(table_ident,
                                     
std::make_shared<TableMetadata>(TableMetadata{
                                         .schemas = {schema},
                                         .current_schema_id = 1,
                                         .current_snapshot_id = 1,
                                         .snapshots = 
{std::make_shared<Snapshot>(Snapshot{
                                             .snapshot_id = 1,
                                             .sequence_number = 1,
                                         })}}),
                                     "s3://location", io, catalog)));
     auto load_table_result = catalog->LoadTable(table_ident);
     ASSERT_THAT(load_table_result, IsOk());
     auto loaded_table = std::move(load_table_result.value());
     ASSERT_EQ(loaded_table->current_snapshot().value()->snapshot_id, 1);
   
     // Mock 2nd call to LoadTable
     EXPECT_CALL(*catalog, LoadTable(::testing::_))
         .WillOnce(::testing::Return(
             std::make_unique<Table>(table_ident,
                                     
std::make_shared<TableMetadata>(TableMetadata{
                                         .schemas = {schema},
                                         .current_schema_id = 1,
                                         .current_snapshot_id = 2,
                                         .snapshots = 
{std::make_shared<Snapshot>(Snapshot{
                                                           .snapshot_id = 1,
                                                           .sequence_number = 1,
                                                       }),
                                                       
std::make_shared<Snapshot>(Snapshot{
                                                           .snapshot_id = 2,
                                                           .sequence_number = 2,
                                                       })}}),
                                     "s3://location", io, catalog)));
     auto refreshed_result = loaded_table->Refresh();
     ASSERT_THAT(refreshed_result, IsOk());
     
ASSERT_EQ(refreshed_result.value()->current_snapshot().value()->snapshot_id, 2);
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to