dannycjones commented on code in PR #2392:
URL: https://github.com/apache/iceberg-rust/pull/2392#discussion_r3577953870
##########
crates/iceberg/src/transaction/mod.rs:
##########
@@ -645,3 +678,131 @@ mod test_row_lineage {
assert_eq!(manifest_file.first_row_id, Some(30));
}
}
+
+#[cfg(test)]
+mod test_commit_against_memory_catalog {
+ //! End-to-end tests for transaction APIs against an in-process
`MemoryCatalog`.
+ //!
+ //! Each test constructs a table inside a fresh memory catalog, builds a
+ //! transaction, commits it, and then asserts that the catalog's view of
the
+ //! table reflects the action's intended metadata change. This complements
+ //! the action-level unit tests in the per-action modules (which only
inspect
+ //! the produced `ActionCommit`) and the mock-catalog tests in this file
+ //! (which exercise the retry loop).
+
+ use crate::Catalog;
+ use crate::memory::tests::new_memory_catalog;
+ use crate::transaction::tests::make_v2_minimal_table_in_catalog;
+ use crate::transaction::{ApplyTransactionAction, Transaction};
+
+ #[tokio::test]
+ async fn test_update_properties_commit_round_trip() {
+ let catalog = new_memory_catalog().await;
+ let table = make_v2_minimal_table_in_catalog(&catalog).await;
+
+ // Sanity: the keys we are about to set are not already present.
+ assert!(table.metadata().properties().get("owner").is_none());
+ assert!(table.metadata().properties().get("team").is_none());
+
+ let tx = Transaction::new(&table);
+ let tx = tx
+ .update_table_properties()
+ .set("owner".to_string(), "iceberg-rust".to_string())
+ .set("team".to_string(), "storage".to_string())
+ .apply(tx)
+ .unwrap();
+
+ let committed = tx.commit(&catalog).await.unwrap();
+
+ // The returned table reflects the commit.
+ assert_eq!(
+ committed.metadata().properties().get("owner"),
+ Some(&"iceberg-rust".to_string())
+ );
+ assert_eq!(
+ committed.metadata().properties().get("team"),
+ Some(&"storage".to_string())
+ );
+
+ // A fresh load from the catalog also sees the properties, confirming
+ // the commit was persisted rather than only mutated in-memory.
+ let reloaded =
catalog.load_table(committed.identifier()).await.unwrap();
+ assert_eq!(
+ reloaded.metadata().properties().get("owner"),
+ Some(&"iceberg-rust".to_string())
+ );
+ assert_eq!(
+ reloaded.metadata().properties().get("team"),
+ Some(&"storage".to_string())
+ );
+
+ // The metadata location must have advanced.
+ assert_ne!(
+ committed.metadata_location(),
+ table.metadata_location(),
+ "commit should advance the metadata pointer"
+ );
+ }
+
+ #[tokio::test]
+ async fn test_update_location_commit_round_trip() {
+ let catalog = new_memory_catalog().await;
+ let table = make_v2_minimal_table_in_catalog(&catalog).await;
+
+ let original_location = table.metadata().location().to_string();
+ let new_location = format!("{original_location}/relocated");
+
+ let tx = Transaction::new(&table);
+ let tx = tx
+ .update_location()
+ .set_location(new_location.clone())
+ .apply(tx)
+ .unwrap();
+
+ let committed = tx.commit(&catalog).await.unwrap();
+
+ assert_eq!(committed.metadata().location(), new_location);
+
+ let reloaded =
catalog.load_table(committed.identifier()).await.unwrap();
+ assert_eq!(reloaded.metadata().location(), new_location);
+ }
+
+ #[tokio::test]
+ async fn test_chained_actions_single_commit() {
Review Comment:
Agree with Andy here, the suggestions here sound useful to add.
@andybradshaw the reviews are very welcome! The more reviewers come from
contributors, the lesser the burden on maintainers.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
