Skip to content

Commit e1de414

Browse files
committed
Add unit tests for PendingUpdate interface
Added comprehensive unit tests for the PendingUpdate interface including: - Basic Apply() success and validation failure scenarios - Commit() success and failure scenarios - Base class polymorphism to verify PendingUpdate works correctly These tests verify the interface compiles correctly and can be properly implemented by subclasses. Also restored builder pattern context in documentation to explain why constructors are protected. Renamed PendingUpdateBase to PendingUpdate and PendingUpdate<T> to PendingUpdateTyped<T> for cleaner API. Now Transaction can use std::vector<std::unique_ptr<PendingUpdate>> which is more intuitive.
1 parent 9f8b0da commit e1de414

File tree

4 files changed

+116
-16
lines changed

4 files changed

+116
-16
lines changed

src/iceberg/pending_update.h

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,17 @@
2828

2929
namespace iceberg {
3030

31-
/// \brief Non-template base class for table metadata changes
31+
/// \brief Base class for table metadata changes using builder pattern
3232
///
3333
/// This base class allows storing different types of PendingUpdate operations
3434
/// in the same collection (e.g., in Transaction). It provides the common Commit()
3535
/// interface that all updates share.
3636
///
3737
/// This matches the Java Iceberg pattern where BaseTransaction stores a
3838
/// List<PendingUpdate> without type parameters.
39-
class ICEBERG_EXPORT PendingUpdateBase {
39+
class ICEBERG_EXPORT PendingUpdate {
4040
public:
41-
virtual ~PendingUpdateBase() = default;
41+
virtual ~PendingUpdate() = default;
4242

4343
/// \brief Apply and commit the pending changes to the table
4444
///
@@ -53,18 +53,18 @@ class ICEBERG_EXPORT PendingUpdateBase {
5353
virtual Status Commit() = 0;
5454

5555
// Non-copyable, movable
56-
PendingUpdateBase(const PendingUpdateBase&) = delete;
57-
PendingUpdateBase& operator=(const PendingUpdateBase&) = delete;
58-
PendingUpdateBase(PendingUpdateBase&&) noexcept = default;
59-
PendingUpdateBase& operator=(PendingUpdateBase&&) noexcept = default;
56+
PendingUpdate(const PendingUpdate&) = delete;
57+
PendingUpdate& operator=(const PendingUpdate&) = delete;
58+
PendingUpdate(PendingUpdate&&) noexcept = default;
59+
PendingUpdate& operator=(PendingUpdate&&) noexcept = default;
6060

6161
protected:
62-
PendingUpdateBase() = default;
62+
PendingUpdate() = default;
6363
};
6464

65-
/// \brief Template class for type-safe table metadata changes
65+
/// \brief Template class for type-safe table metadata changes using builder pattern
6666
///
67-
/// PendingUpdate extends PendingUpdateBase with a type-safe Apply() method that
67+
/// PendingUpdateTyped extends PendingUpdate with a type-safe Apply() method that
6868
/// returns the specific result type for each operation. Subclasses implement
6969
/// specific types of table updates such as schema changes, property updates, or
7070
/// snapshot-producing operations like appends and deletes.
@@ -74,9 +74,9 @@ class ICEBERG_EXPORT PendingUpdateBase {
7474
///
7575
/// \tparam T The type of result returned by Apply()
7676
template <typename T>
77-
class ICEBERG_EXPORT PendingUpdate : public PendingUpdateBase {
77+
class ICEBERG_EXPORT PendingUpdateTyped : public PendingUpdate {
7878
public:
79-
~PendingUpdate() override = default;
79+
~PendingUpdateTyped() override = default;
8080

8181
/// \brief Apply the pending changes and return the uncommitted result
8282
///
@@ -88,7 +88,7 @@ class ICEBERG_EXPORT PendingUpdate : public PendingUpdateBase {
8888
virtual Result<T> Apply() = 0;
8989

9090
protected:
91-
PendingUpdate() = default;
91+
PendingUpdateTyped() = default;
9292
};
9393

9494
} // namespace iceberg

src/iceberg/test/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ add_iceberg_test(table_test
8383
json_internal_test.cc
8484
table_test.cc
8585
schema_json_test.cc
86-
table_metadata_builder_test.cc)
86+
table_metadata_builder_test.cc
87+
pending_update_test.cc)
8788

8889
add_iceberg_test(expression_test
8990
SOURCES
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#include "iceberg/pending_update.h"
21+
22+
#include <gtest/gtest.h>
23+
24+
#include "iceberg/result.h"
25+
#include "iceberg/test/matchers.h"
26+
27+
namespace iceberg {
28+
29+
// Mock implementation for testing the interface
30+
class MockSnapshot {};
31+
32+
class MockPendingUpdate : public PendingUpdateTyped<MockSnapshot> {
33+
public:
34+
MockPendingUpdate() = default;
35+
36+
Result<MockSnapshot> Apply() override {
37+
if (should_fail_) {
38+
return ValidationFailed("Mock validation failed");
39+
}
40+
return MockSnapshot{};
41+
}
42+
43+
Status Commit() override {
44+
if (should_fail_commit_) {
45+
return CommitFailed("Mock commit failed");
46+
}
47+
apply_called_ = true;
48+
commit_called_ = true;
49+
return {};
50+
}
51+
52+
void SetShouldFail(bool fail) { should_fail_ = fail; }
53+
void SetShouldFailCommit(bool fail) { should_fail_commit_ = fail; }
54+
bool ApplyCalled() const { return apply_called_; }
55+
bool CommitCalled() const { return commit_called_; }
56+
57+
private:
58+
bool should_fail_ = false;
59+
bool should_fail_commit_ = false;
60+
bool apply_called_ = false;
61+
bool commit_called_ = false;
62+
};
63+
64+
TEST(PendingUpdateTest, ApplySuccess) {
65+
MockPendingUpdate update;
66+
auto result = update.Apply();
67+
EXPECT_THAT(result, IsOk());
68+
}
69+
70+
TEST(PendingUpdateTest, ApplyValidationFailed) {
71+
MockPendingUpdate update;
72+
update.SetShouldFail(true);
73+
auto result = update.Apply();
74+
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
75+
EXPECT_THAT(result, HasErrorMessage("Mock validation failed"));
76+
}
77+
78+
TEST(PendingUpdateTest, CommitSuccess) {
79+
MockPendingUpdate update;
80+
auto status = update.Commit();
81+
EXPECT_THAT(status, IsOk());
82+
EXPECT_TRUE(update.CommitCalled());
83+
}
84+
85+
TEST(PendingUpdateTest, CommitFailed) {
86+
MockPendingUpdate update;
87+
update.SetShouldFailCommit(true);
88+
auto status = update.Commit();
89+
EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
90+
EXPECT_THAT(status, HasErrorMessage("Mock commit failed"));
91+
}
92+
93+
TEST(PendingUpdateTest, BaseClassPolymorphism) {
94+
std::unique_ptr<PendingUpdate> base_ptr = std::make_unique<MockPendingUpdate>();
95+
auto status = base_ptr->Commit();
96+
EXPECT_THAT(status, IsOk());
97+
}
98+
99+
} // namespace iceberg

src/iceberg/type_fwd.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,9 @@ class TableRequirement;
154154
class TableMetadataBuilder;
155155
class TableUpdateContext;
156156

157-
class PendingUpdateBase;
158-
template <typename T>
159157
class PendingUpdate;
158+
template <typename T>
159+
class PendingUpdateTyped;
160160

161161
/// ----------------------------------------------------------------------------
162162
/// TODO: Forward declarations below are not added yet.

0 commit comments

Comments
 (0)