Skip to content

Commit b4a522b

Browse files
teskjeteskje
committed
catalog-protos: adjust unit tests to check Rust object defs
1 parent fcf1c87 commit b4a522b

File tree

4 files changed

+60
-80
lines changed

4 files changed

+60
-80
lines changed

‎src/catalog-protos/src/lib.rs‎

Lines changed: 18 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -48,29 +48,25 @@ pub const MIN_CATALOG_VERSION: u64 = 67;
4848
mod tests {
4949
use std::collections::BTreeSet;
5050
use std::fs;
51-
use std::io::{BufRead, BufReader};
51+
use std::path::PathBuf;
5252

5353
use crate::{CATALOG_VERSION, MIN_CATALOG_VERSION};
5454

55-
// Note: Feel free to update this path if the protos move.
56-
const PROTO_DIRECTORY: &str = "protos";
57-
5855
#[mz_ore::test]
5956
fn test_assert_snapshots_exist() {
60-
// Get all of the files in the snapshot directory, with the `.proto` extension.
61-
let mut filenames: BTreeSet<_> = fs::read_dir(PROTO_DIRECTORY)
62-
.expect("failed to read protos dir")
57+
let src_dir: PathBuf = [env!("CARGO_MANIFEST_DIR"), "src"].iter().collect();
58+
59+
// Get all the versioned object definition files.
60+
let mut filenames: BTreeSet<_> = fs::read_dir(src_dir)
61+
.expect("failed to read src dir")
6362
.map(|entry| entry.expect("failed to read dir entry").file_name())
6463
.map(|filename| filename.to_str().expect("utf8").to_string())
65-
.filter(|filename| filename.ends_with("proto"))
64+
.filter(|filename| filename.starts_with("objects_v"))
6665
.collect();
6766

68-
// Assert objects.proto exists.
69-
assert!(filenames.remove("objects.proto"));
70-
7167
// Assert snapshots exist for all of the versions we support.
7268
for version in MIN_CATALOG_VERSION..=CATALOG_VERSION {
73-
let filename = format!("objects_v{version}.proto");
69+
let filename = format!("objects_v{version}.rs");
7470
assert!(
7571
filenames.remove(&filename),
7672
"Missing snapshot for v{version}."
@@ -90,39 +86,24 @@ mod tests {
9086
// Assert there aren't any extra snapshots.
9187
assert!(
9288
filenames.is_empty(),
93-
"Found snapshots for unsupported catalog versions {filenames:?}.\nIf you just increased `MIN_CATALOG_VERSION`, then please delete the old snapshots. If you created a new snapshot, please bump `CATALOG_VERSION`."
89+
"Found snapshots for unsupported catalog versions {filenames:?}.\n\
90+
If you just increased `MIN_CATALOG_VERSION`, then please delete the old snapshots. \
91+
If you created a new snapshot, please bump `CATALOG_VERSION`."
9492
);
9593
}
9694

9795
#[mz_ore::test]
9896
fn test_assert_current_snapshot() {
99-
// Read the content from both files.
100-
let current = fs::File::open(format!("{PROTO_DIRECTORY}/objects.proto"))
101-
.map(BufReader::new)
102-
.expect("read current");
103-
let snapshot = fs::File::open(format!(
104-
"{PROTO_DIRECTORY}/objects_v{CATALOG_VERSION}.proto"
105-
))
106-
.map(BufReader::new)
107-
.expect("read snapshot");
97+
let src_dir: PathBuf = [env!("CARGO_MANIFEST_DIR"), "src"].iter().collect();
98+
let current_rs = src_dir.join("objects.rs");
99+
let snapshot_rs = src_dir.join(format!("objects_v{CATALOG_VERSION}.rs"));
108100

109-
// Read in all of the lines so we can compare the content of †he files.
110-
let current: Vec<_> = current
111-
.lines()
112-
.map(|r| r.expect("failed to read line from current"))
113-
// Filter out the package name, since we expect that to be different.
114-
.filter(|line| line != "package objects;")
115-
.collect();
116-
let snapshot: Vec<_> = snapshot
117-
.lines()
118-
.map(|r| r.expect("failed to read line from current"))
119-
// Filter out the package name, since we expect that to be different.
120-
.filter(|line| line != &format!("package objects_v{CATALOG_VERSION};"))
121-
.collect();
101+
let current = fs::read_to_string(current_rs).expect("read current");
102+
let snapshot = fs::read_to_string(snapshot_rs).expect("read snapshot");
122103

123-
// Note: objects.proto and objects_v<CATALOG_VERSION>.proto should be exactly the same. The
104+
// Note: objects.rs and objects_v<CATALOG_VERSION>.rs should be exactly the same. The
124105
// reason being, when bumping the catalog to the next version, CATALOG_VERSION + 1, we need a
125-
// snapshot to migrate _from_, which should be a snapshot of how the protos are today.
106+
// snapshot to migrate _from_, which should be a snapshot of how the types are today.
126107
// Hence why the two files should be exactly the same.
127108
similar_asserts::assert_eq!(current, snapshot);
128109
}

‎src/catalog/src/durable/upgrade/tests.rs‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ use mz_storage_types::sources::SourceData;
1919
use crate::durable::objects::state_update::StateUpdateKindJson;
2020
use crate::durable::upgrade::AllVersionsStateUpdateKind;
2121

22-
const PROTO_DIRECTORY: &str = "../catalog-protos/protos";
23-
const PROTO_EXT: &str = "proto";
22+
const PROTO_DIRECTORY: &str = "../catalog-protos/src";
23+
const PROTO_EXT: &str = "rs";
2424

2525
static SNAPSHOT_DIRECTORY: &str = "src/durable/upgrade/snapshots";
2626
const SNAPSHOT_EXT: &str = "txt";
@@ -29,28 +29,25 @@ const SNAPSHOT_EXT: &str = "txt";
2929
#[cfg_attr(miri, ignore)] // too slow
3030
fn test_proto_serialization_stability() {
3131
let protos: BTreeSet<_> = read_file_names(PROTO_DIRECTORY, PROTO_EXT)
32-
// Remove `objects.proto`.
33-
//
34-
// `objects.proto` is allowed to change and we don't have a good
35-
// mechanism to force people to update `objects.txt` any time `objects.proto` changes. To
36-
// avoid this rot we just don't test the contents of `objects.proto`. Additionally,
37-
// `objects.proto` will always be identical to the most recent `objects_vX.proto`.
38-
.filter(|name| name != "objects")
32+
.filter(|name| name.starts_with("objects_v"))
3933
.collect();
4034

4135
let snapshot_files: BTreeSet<_> = read_file_names(SNAPSHOT_DIRECTORY, SNAPSHOT_EXT).collect();
4236

4337
let unknown_snapshots: Vec<_> = snapshot_files.difference(&protos).collect();
4438
if !unknown_snapshots.is_empty() {
4539
panic!(
46-
"Have snapshots, but no proto files on disk? If a .proto file was deleted, then the .txt snapshot file must be deleted too. {unknown_snapshots:#?}"
40+
"Have snapshots, but no proto files on disk? \
41+
If an objects_v*.rs file was deleted, then the .txt snapshot file must be deleted \
42+
too. {unknown_snapshots:#?}"
4743
);
4844
}
4945

5046
let unencoded_protos: Vec<_> = protos.difference(&snapshot_files).collect();
5147
if !unencoded_protos.is_empty() {
5248
panic!(
53-
"Missing encodings for some proto objects, try generating them with `generate_missing_encodings`. {unencoded_protos:#?}"
49+
"Missing encodings for some proto objects, try generating them with \
50+
`generate_missing_encodings`. {unencoded_protos:#?}"
5451
);
5552
}
5653

‎src/catalog/tests/snapshots/debug__opened_trace.snap‎

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
---
22
source: src/catalog/tests/debug.rs
3+
assertion_line: 233
34
expression: test_trace
45
---
56
Trace {
@@ -12,8 +13,8 @@ Trace {
1213
V1(
1314
AuditLogEventV1 {
1415
id: 1,
15-
event_type: Grant,
16-
object_type: Cluster,
16+
event_type: 4,
17+
object_type: 1,
1718
user: None,
1819
occurred_at: Some(
1920
EpochMillis {
@@ -49,8 +50,8 @@ Trace {
4950
V1(
5051
AuditLogEventV1 {
5152
id: 2,
52-
event_type: Grant,
53-
object_type: Database,
53+
event_type: 4,
54+
object_type: 4,
5455
user: None,
5556
occurred_at: Some(
5657
EpochMillis {
@@ -86,8 +87,8 @@ Trace {
8687
V1(
8788
AuditLogEventV1 {
8889
id: 3,
89-
event_type: Grant,
90-
object_type: Schema,
90+
event_type: 4,
91+
object_type: 10,
9192
user: None,
9293
occurred_at: Some(
9394
EpochMillis {
@@ -123,8 +124,8 @@ Trace {
123124
V1(
124125
AuditLogEventV1 {
125126
id: 4,
126-
event_type: Grant,
127-
object_type: Type,
127+
event_type: 4,
128+
object_type: 14,
128129
user: None,
129130
occurred_at: Some(
130131
EpochMillis {
@@ -160,8 +161,8 @@ Trace {
160161
V1(
161162
AuditLogEventV1 {
162163
id: 5,
163-
event_type: Create,
164-
object_type: Database,
164+
event_type: 1,
165+
object_type: 4,
165166
user: None,
166167
occurred_at: Some(
167168
EpochMillis {
@@ -194,8 +195,8 @@ Trace {
194195
V1(
195196
AuditLogEventV1 {
196197
id: 6,
197-
event_type: Grant,
198-
object_type: Database,
198+
event_type: 4,
199+
object_type: 4,
199200
user: None,
200201
occurred_at: Some(
201202
EpochMillis {
@@ -230,8 +231,8 @@ Trace {
230231
V1(
231232
AuditLogEventV1 {
232233
id: 7,
233-
event_type: Create,
234-
object_type: Schema,
234+
event_type: 1,
235+
object_type: 10,
235236
user: None,
236237
occurred_at: Some(
237238
EpochMillis {
@@ -269,8 +270,8 @@ Trace {
269270
V1(
270271
AuditLogEventV1 {
271272
id: 8,
272-
event_type: Create,
273-
object_type: NetworkPolicy,
273+
event_type: 1,
274+
object_type: 18,
274275
user: None,
275276
occurred_at: Some(
276277
EpochMillis {
@@ -303,8 +304,8 @@ Trace {
303304
V1(
304305
AuditLogEventV1 {
305306
id: 9,
306-
event_type: Create,
307-
object_type: Cluster,
307+
event_type: 1,
308+
object_type: 1,
308309
user: None,
309310
occurred_at: Some(
310311
EpochMillis {
@@ -337,8 +338,8 @@ Trace {
337338
V1(
338339
AuditLogEventV1 {
339340
id: 10,
340-
event_type: Grant,
341-
object_type: Cluster,
341+
event_type: 4,
342+
object_type: 1,
342343
user: None,
343344
occurred_at: Some(
344345
EpochMillis {
@@ -373,8 +374,8 @@ Trace {
373374
V1(
374375
AuditLogEventV1 {
375376
id: 11,
376-
event_type: Create,
377-
object_type: ClusterReplica,
377+
event_type: 1,
378+
object_type: 2,
378379
user: None,
379380
occurred_at: Some(
380381
EpochMillis {
@@ -426,8 +427,8 @@ Trace {
426427
V1(
427428
AuditLogEventV1 {
428429
id: 12,
429-
event_type: Grant,
430-
object_type: System,
430+
event_type: 4,
431+
object_type: 16,
431432
user: None,
432433
occurred_at: Some(
433434
EpochMillis {
@@ -827,7 +828,7 @@ Trace {
827828
),
828829
database_id: None,
829830
schema_id: None,
830-
object_type: Type,
831+
object_type: 7,
831832
grantee: Some(
832833
RoleId {
833834
value: Some(
@@ -865,7 +866,7 @@ Trace {
865866
),
866867
database_id: None,
867868
schema_id: None,
868-
object_type: Cluster,
869+
object_type: 9,
869870
grantee: Some(
870871
RoleId {
871872
value: Some(
@@ -903,7 +904,7 @@ Trace {
903904
),
904905
database_id: None,
905906
schema_id: None,
906-
object_type: Database,
907+
object_type: 13,
907908
grantee: Some(
908909
RoleId {
909910
value: Some(
@@ -941,7 +942,7 @@ Trace {
941942
),
942943
database_id: None,
943944
schema_id: None,
944-
object_type: Schema,
945+
object_type: 14,
945946
grantee: Some(
946947
RoleId {
947948
value: Some(

‎src/catalog/tests/snapshots/open__initial_snapshot.snap‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
---
22
source: src/catalog/tests/open.rs
3+
assertion_line: 518
34
expression: test_snapshot
45
---
56
Snapshot {
@@ -1479,7 +1480,7 @@ Snapshot {
14791480
),
14801481
database_id: None,
14811482
schema_id: None,
1482-
object_type: Type,
1483+
object_type: 7,
14831484
grantee: Some(
14841485
RoleId {
14851486
value: Some(
@@ -1508,7 +1509,7 @@ Snapshot {
15081509
),
15091510
database_id: None,
15101511
schema_id: None,
1511-
object_type: Cluster,
1512+
object_type: 9,
15121513
grantee: Some(
15131514
RoleId {
15141515
value: Some(
@@ -1537,7 +1538,7 @@ Snapshot {
15371538
),
15381539
database_id: None,
15391540
schema_id: None,
1540-
object_type: Database,
1541+
object_type: 13,
15411542
grantee: Some(
15421543
RoleId {
15431544
value: Some(
@@ -1566,7 +1567,7 @@ Snapshot {
15661567
),
15671568
database_id: None,
15681569
schema_id: None,
1569-
object_type: Schema,
1570+
object_type: 14,
15701571
grantee: Some(
15711572
RoleId {
15721573
value: Some(

0 commit comments

Comments
 (0)