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