From c730db13bf822326c4afdf175779f9eebc7ee859 Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Fri, 21 Nov 2025 17:32:52 -0500 Subject: [PATCH 01/20] tyler/fix-on-conflict --- crates/cli/src/subcommands/dev.rs | 15 ++++--- crates/cli/src/subcommands/publish.rs | 60 ++++++++++++--------------- 2 files changed, 33 insertions(+), 42 deletions(-) diff --git a/crates/cli/src/subcommands/dev.rs b/crates/cli/src/subcommands/dev.rs index c8541ba76ab..608d1ac813e 100644 --- a/crates/cli/src/subcommands/dev.rs +++ b/crates/cli/src/subcommands/dev.rs @@ -413,15 +413,14 @@ async fn generate_build_and_publish( ClearMode::OnConflict => "on-conflict", }; let mut publish_args = vec![ - "publish", - database_name, - "--project-path", - project_path_str, - "--yes", - "--delete-data", - clear_flag, + "publish".to_string(), + database_name.to_string(), + "--project-path".to_string(), + project_path_str.to_string(), + "--yes".to_string(), + format!("--delete-data={}", clear_flag), ]; - publish_args.extend_from_slice(&["--server", server]); + publish_args.extend_from_slice(&["--server".to_string(), server.to_string()]); let publish_cmd = publish::cli(); let publish_matches = publish_cmd diff --git a/crates/cli/src/subcommands/publish.rs b/crates/cli/src/subcommands/publish.rs index e60bcb96840..f4518258868 100644 --- a/crates/cli/src/subcommands/publish.rs +++ b/crates/cli/src/subcommands/publish.rs @@ -175,46 +175,26 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E let domain = percent_encoding::percent_encode(name_or_identity.as_bytes(), encode_set); let mut builder = client.put(format!("{database_host}/v1/database/{domain}")); - if clear_database != ClearMode::Always { - builder = apply_pre_publish_if_needed( - builder, - &client, - &database_host, - &domain.to_string(), - host_type, - &program_bytes, - &auth_header, - clear_database, - force_break_clients, - force, - ) - .await?; - } + builder = apply_pre_publish_if_needed( + builder, + &client, + &database_host, + name_or_identity, + &domain.to_string(), + host_type, + &program_bytes, + &auth_header, + clear_database, + force_break_clients, + force, + ) + .await?; builder } else { client.post(format!("{database_host}/v1/database")) }; - if clear_database == ClearMode::Always || clear_database == ClearMode::OnConflict { - // Note: `name_or_identity` should be set, because it is `required` in the CLI arg config. - println!( - "This will DESTROY the current {} module, and ALL corresponding data.", - name_or_identity.unwrap() - ); - if !y_or_n( - force, - format!( - "Are you sure you want to proceed? [deleting {}]", - name_or_identity.unwrap() - ) - .as_str(), - )? { - println!("Aborting"); - return Ok(()); - } - builder = builder.query(&[("clear", true)]); - } if let Some(n) = num_replicas { eprintln!("WARNING: Use of unstable option `--num-replicas`.\n"); builder = builder.query(&[("num_replicas", *n)]); @@ -334,6 +314,7 @@ async fn apply_pre_publish_if_needed( mut builder: reqwest::RequestBuilder, client: &reqwest::Client, base_url: &str, + name_or_identity: &str, domain: &String, host_type: &str, program_bytes: &[u8], @@ -367,6 +348,17 @@ async fn apply_pre_publish_if_needed( println!("{}", manual.reason); println!("Proceeding with database clear due to --delete-data=always."); } + println!( + "This will DESTROY the current {} module, and ALL corresponding data.", + name_or_identity + ); + if !y_or_n( + force, + format!("Are you sure you want to proceed? [deleting {}]", name_or_identity).as_str(), + )? { + anyhow::bail!("Aborting"); + } + builder = builder.query(&[("clear", true)]); } PrePublishResult::AutoMigrate(auto) => { println!("{}", auto.migrate_plan); From 9dcf58baffaff854ad9e74d5ca2ff42b5142510e Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Sat, 22 Nov 2025 22:14:47 -0500 Subject: [PATCH 02/20] Added automated testing --- Cargo.lock | 81 +++++++++++++ crates/cli/Cargo.toml | 5 + crates/cli/src/subcommands/build.rs | 10 +- crates/cli/src/subcommands/dev.rs | 2 +- crates/cli/src/subcommands/publish.rs | 18 ++- crates/cli/src/tasks/mod.rs | 6 +- crates/cli/src/tasks/rust.rs | 14 ++- crates/cli/tests/publish.rs | 164 ++++++++++++++++++++++++++ crates/cli/tests/server.rs | 17 +++ crates/cli/tests/util.rs | 150 +++++++++++++++++++++++ crates/testing/src/modules.rs | 1 + modules/module-test/Cargo.toml | 3 + modules/module-test/src/lib.rs | 22 ++++ 13 files changed, 486 insertions(+), 7 deletions(-) create mode 100644 crates/cli/tests/publish.rs create mode 100644 crates/cli/tests/server.rs create mode 100644 crates/cli/tests/util.rs diff --git a/Cargo.lock b/Cargo.lock index f18ad1a7985..6fc1930d0b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -219,6 +219,21 @@ version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" +[[package]] +name = "assert_cmd" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bcbb6924530aa9e0432442af08bbcafdad182db80d2e560da42a6d442535bf85" +dependencies = [ + "anstyle", + "bstr", + "libc", + "predicates", + "predicates-core", + "predicates-tree", + "wait-timeout", +] + [[package]] name = "async-scoped" version = "0.9.0" @@ -623,6 +638,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "63044e1ae8e69f3b5a92c736ca6269b8d12fa7efe39bf34ddb06d102cf0e2cab" dependencies = [ "memchr", + "regex-automata", "serde", ] @@ -1706,6 +1722,12 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" +[[package]] +name = "difflib" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6184e33543162437515c2e2b48714794e37845ec9851711914eec9d308f6ebe8" + [[package]] name = "digest" version = "0.10.7" @@ -2161,6 +2183,15 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "float-cmp" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b09cf3155332e944990140d967ff5eceb70df778b34f77d8075db46e4704e6d8" +dependencies = [ + "num-traits", +] + [[package]] name = "flume" version = "0.11.1" @@ -4004,6 +4035,12 @@ version = "0.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "610a5acd306ec67f907abe5567859a3c693fb9886eb1f012ab8f2a47bef3db51" +[[package]] +name = "normalize-line-endings" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" + [[package]] name = "notify" version = "7.0.0" @@ -5168,6 +5205,15 @@ version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f84267b20a16ea918e43c6a88433c2d54fa145c92a811b5b047ccbe153674483" +[[package]] +name = "portpicker" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "be97d76faf1bfab666e1375477b23fde79eccf0276e9b63b92a39d676a889ba9" +dependencies = [ + "rand 0.8.5", +] + [[package]] name = "postcard" version = "1.1.3" @@ -5266,6 +5312,36 @@ dependencies = [ "zerocopy", ] +[[package]] +name = "predicates" +version = "3.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a5d19ee57562043d37e82899fade9a22ebab7be9cef5026b07fda9cdd4293573" +dependencies = [ + "anstyle", + "difflib", + "float-cmp", + "normalize-line-endings", + "predicates-core", + "regex", +] + +[[package]] +name = "predicates-core" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "727e462b119fe9c93fd0eb1429a5f7647394014cf3c04ab2c0350eeb09095ffa" + +[[package]] +name = "predicates-tree" +version = "1.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72dd2d6d381dfb73a193c7fca536518d7caee39fc8503f74e7dc0be0531b425c" +dependencies = [ + "predicates-core", + "termtree", +] + [[package]] name = "pretty_assertions" version = "1.4.1" @@ -5894,6 +5970,7 @@ dependencies = [ "base64 0.22.1", "bytes", "encoding_rs", + "futures-channel", "futures-core", "futures-util", "h2 0.4.12", @@ -7222,6 +7299,7 @@ name = "spacetimedb-cli" version = "1.9.0" dependencies = [ "anyhow", + "assert_cmd", "base64 0.21.7", "bytes", "cargo_metadata", @@ -7236,6 +7314,7 @@ dependencies = [ "email_address", "flate2", "fs-err", + "fs_extra", "futures", "git2", "http 1.3.1", @@ -7246,6 +7325,8 @@ dependencies = [ "names", "notify 7.0.0", "percent-encoding", + "portpicker", + "predicates", "pretty_assertions", "quick-xml 0.31.0", "regex", diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 5f93a58a9d2..58891a78a26 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -86,6 +86,11 @@ notify.workspace = true [dev-dependencies] pretty_assertions.workspace = true +fs_extra.workspace = true +assert_cmd = "2" +predicates = "3" +portpicker = "0.1" +reqwest = { version = "0.12", features = ["blocking", "json"] } [target.'cfg(not(target_env = "msvc"))'.dependencies] tikv-jemallocator = { workspace = true } diff --git a/crates/cli/src/subcommands/build.rs b/crates/cli/src/subcommands/build.rs index e0c31b20ed1..411056cd405 100644 --- a/crates/cli/src/subcommands/build.rs +++ b/crates/cli/src/subcommands/build.rs @@ -22,6 +22,13 @@ pub fn cli() -> clap::Command { .default_value("src") .help("The directory to lint for nonfunctional print statements. If set to the empty string, skips linting.") ) + .arg( + Arg::new("features") + .long("features") + .value_parser(clap::value_parser!(OsString)) + .required(false) + .help("Additional features to pass to the build process (e.g. `--features feature1,feature2` for Rust modules).") + ) .arg( Arg::new("debug") .long("debug") @@ -33,6 +40,7 @@ pub fn cli() -> clap::Command { pub async fn exec(_config: Config, args: &ArgMatches) -> Result<(PathBuf, &'static str), anyhow::Error> { let project_path = args.get_one::("project_path").unwrap(); + let features = args.get_one::("features"); let lint_dir = args.get_one::("lint_dir").unwrap(); let lint_dir = if lint_dir.is_empty() { None @@ -56,7 +64,7 @@ pub async fn exec(_config: Config, args: &ArgMatches) -> Result<(PathBuf, &'stat )); } - let result = crate::tasks::build(project_path, lint_dir.as_deref(), build_debug)?; + let result = crate::tasks::build(project_path, lint_dir.as_deref(), build_debug, features)?; println!("Build finished successfully."); Ok(result) diff --git a/crates/cli/src/subcommands/dev.rs b/crates/cli/src/subcommands/dev.rs index 608d1ac813e..ca8271eb235 100644 --- a/crates/cli/src/subcommands/dev.rs +++ b/crates/cli/src/subcommands/dev.rs @@ -388,7 +388,7 @@ async fn generate_build_and_publish( println!("{}", "Building...".cyan()); let (_path_to_program, _host_type) = - tasks::build(spacetimedb_dir, Some(Path::new("src")), false).context("Failed to build project")?; + tasks::build(spacetimedb_dir, Some(Path::new("src")), false, None).context("Failed to build project")?; println!("{}", "Build complete!".green()); println!("{}", "Generating module bindings...".cyan()); diff --git a/crates/cli/src/subcommands/publish.rs b/crates/cli/src/subcommands/publish.rs index f4518258868..044344676b5 100644 --- a/crates/cli/src/subcommands/publish.rs +++ b/crates/cli/src/subcommands/publish.rs @@ -67,6 +67,7 @@ pub fn cli() -> clap::Command { .arg( Arg::new("break_clients") .long("break-clients") + .alias("yes-break-clients") .action(SetTrue) .help("Allow breaking changes when publishing to an existing database identity. This will force publish even if it will break existing clients, but will NOT force publish if it would cause deletion of any data in the database. See --yes and --delete-data for details.") ) @@ -361,9 +362,22 @@ async fn apply_pre_publish_if_needed( builder = builder.query(&[("clear", true)]); } PrePublishResult::AutoMigrate(auto) => { + if clear_database == ClearMode::Always { + println!("Auto-migration, does NOT require clearing the database, but proceeding with database clear due to --delete-data=always."); + println!( + "This will DESTROY the current {} module, and ALL corresponding data.", + name_or_identity + ); + if !y_or_n( + force, + format!("Are you sure you want to proceed? [deleting {}]", name_or_identity).as_str(), + )? { + anyhow::bail!("Aborting"); + } + builder = builder.query(&[("clear", true)]); + return Ok(builder); + } println!("{}", auto.migrate_plan); - // We only arrive here if you have not specified ClearMode::Always AND there was no - // conflict that required manual migration. if auto.break_clients && !y_or_n( force_break_clients || force, diff --git a/crates/cli/src/tasks/mod.rs b/crates/cli/src/tasks/mod.rs index a88f10a9b0b..26e6425e2af 100644 --- a/crates/cli/src/tasks/mod.rs +++ b/crates/cli/src/tasks/mod.rs @@ -13,10 +13,14 @@ pub fn build( project_path: &Path, lint_dir: Option<&Path>, build_debug: bool, + features: Option<&std::ffi::OsString>, ) -> anyhow::Result<(PathBuf, &'static str)> { let lang = util::detect_module_language(project_path)?; + if features.is_some() && lang != ModuleLanguage::Rust { + anyhow::bail!("The --features option is only supported for Rust modules."); + } let output_path = match lang { - ModuleLanguage::Rust => build_rust(project_path, lint_dir, build_debug), + ModuleLanguage::Rust => build_rust(project_path, features, lint_dir, build_debug), ModuleLanguage::Csharp => build_csharp(project_path, build_debug), ModuleLanguage::Javascript => build_javascript(project_path, build_debug), }?; diff --git a/crates/cli/src/tasks/rust.rs b/crates/cli/src/tasks/rust.rs index 9765d1b424d..9b6e52871b3 100644 --- a/crates/cli/src/tasks/rust.rs +++ b/crates/cli/src/tasks/rust.rs @@ -23,7 +23,7 @@ fn cargo_cmd(subcommand: &str, build_debug: bool, args: &[&str]) -> duct::Expres ) } -pub(crate) fn build_rust(project_path: &Path, lint_dir: Option<&Path>, build_debug: bool) -> anyhow::Result { +pub(crate) fn build_rust(project_path: &Path, features: Option<&std::ffi::OsString>, lint_dir: Option<&Path>, build_debug: bool) -> anyhow::Result { // Make sure that we have the wasm target installed if !has_wasm32_target() { if has_rust_up() { @@ -75,7 +75,17 @@ pub(crate) fn build_rust(project_path: &Path, lint_dir: Option<&Path>, build_deb ); } - let reader = cargo_cmd("build", build_debug, &["--message-format=json-render-diagnostics"]) + let mut args = if let Some(features) = features { + vec![format!("--features={}", features.to_string_lossy())] + } else { + vec![] + }; + args.push("--message-format=json-render-diagnostics".to_string()); + + // Convert Vec to Vec<&str> + let args_str: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + + let reader = cargo_cmd("build", build_debug, &args_str) .dir(project_path) .reader()?; diff --git a/crates/cli/tests/publish.rs b/crates/cli/tests/publish.rs new file mode 100644 index 00000000000..a57d9d5bf15 --- /dev/null +++ b/crates/cli/tests/publish.rs @@ -0,0 +1,164 @@ +mod util; + +use crate::util::SpacetimeDbGuard; +use assert_cmd::cargo::cargo_bin_cmd; + +#[test] +fn cli_can_publish_spacetimedb_on_disk() { + let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); + + // Workspace root for `cargo run -p ...` + let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; + // dir = /modules/quickstart-chat + let dir = workspace_dir.join("modules").join("quickstart-chat"); + + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args(["publish", "--server", &spacetime.host_url.to_string(), "foobar"]) + .current_dir(dir.clone()) + .assert() + .success(); + + // Can republish without error to the same name + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args(["publish", "--server", &spacetime.host_url.to_string(), "foobar"]) + .current_dir(dir) + .assert() + .success(); +} + +#[test] +fn cli_can_publish_with_automigration_change() { + let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); + + // Workspace root for `cargo run -p ...` + let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; + let dir = workspace_dir.join("modules").join("module-test"); + + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "publish", + "--server", + &spacetime.host_url.to_string(), + "automigration-test", + ]) + .current_dir(dir.clone()) + .assert() + .success(); + + // Can republish with automigration change + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "publish", + "--build-options=--features test-add-column", + "--server", + &spacetime.host_url.to_string(), + "--yes-break-clients", + "automigration-test", + ]) + .current_dir(dir) + .assert() + .success(); +} + +#[test] +fn cli_cannot_publish_breaking_change_without_flag() { + let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); + + // Workspace root for `cargo run -p ...` + let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; + let dir = workspace_dir.join("modules").join("module-test"); + + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "publish", + "--server", + &spacetime.host_url.to_string(), + "breaking-change-test", + ]) + .current_dir(dir.clone()) + .assert() + .success(); + + // Cannot republish with breaking change without flag + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "publish", + "--build-options=--features test-remove-table", + "--server", + &spacetime.host_url.to_string(), + "breaking-change-test", + ]) + .current_dir(dir) + .assert() + .failure(); +} + +#[test] +fn cli_can_publish_breaking_change_with_delete_data_flag() { + let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); + + // Workspace root for `cargo run -p ...` + let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; + let dir = workspace_dir.join("modules").join("module-test"); + + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "publish", + "--server", + &spacetime.host_url.to_string(), + "breaking-change-delete-data-test", + ]) + .current_dir(dir.clone()) + .assert() + .success(); + + // Can republish with breaking change with --delete-data flag + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "publish", + "--build-options=--features test-remove-table", + "--server", + &spacetime.host_url.to_string(), + "--delete-data", + "--yes", + "breaking-change-delete-data-test", + ]) + .current_dir(dir) + .assert() + .success(); +} + +#[test] +fn cli_can_publish_breaking_change_with_on_conflict_flag() { + let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); + + // Workspace root for `cargo run -p ...` + let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; + let dir = workspace_dir.join("modules").join("module-test"); + + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "publish", + "--server", + &spacetime.host_url.to_string(), + "breaking-change-on-conflict-test", + ]) + .current_dir(dir.clone()) + .assert() + .success(); + + // Can republish with breaking change with --on-conflict=delete-data flag + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "publish", + "--build-options=--features test-remove-table", + "--server", + &spacetime.host_url.to_string(), + "--delete-data=on-conflict", + "--yes", + "breaking-change-on-conflict-test", + ]) + .current_dir(dir) + .assert() + .success(); +} diff --git a/crates/cli/tests/server.rs b/crates/cli/tests/server.rs new file mode 100644 index 00000000000..75bb44589a7 --- /dev/null +++ b/crates/cli/tests/server.rs @@ -0,0 +1,17 @@ +mod util; + +use assert_cmd::cargo::cargo_bin_cmd; +use crate::util::SpacetimeDbGuard; + +#[test] +fn cli_can_ping_spacetimedb_on_disk() { + let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "server", + "ping", + &spacetime.host_url.to_string(), + ]) + .assert() + .success(); +} \ No newline at end of file diff --git a/crates/cli/tests/util.rs b/crates/cli/tests/util.rs new file mode 100644 index 00000000000..3f9d7a93a49 --- /dev/null +++ b/crates/cli/tests/util.rs @@ -0,0 +1,150 @@ + +use std::{ + env, io::{BufReader, BufRead}, net::SocketAddr, process::{Child, Command, Stdio}, sync::{Arc, Mutex}, thread::{self, sleep}, time::{Duration, Instant} +}; + +use reqwest::blocking::Client; + +fn find_free_port() -> u16 { + portpicker::pick_unused_port().expect("no free ports available") +} + +pub struct SpacetimeDbGuard { + pub child: Child, + pub host_url: String, + pub logs: Arc>, +} + +impl SpacetimeDbGuard { + + /// Start `spacetimedb` in a temporary data directory via: + /// cargo run -p spacetimedb-cli -- start --data-dir --listen-addr + pub fn spawn_in_temp_data_dir() -> Self { + let temp_dir = tempfile::tempdir().expect("failed to create temp dir"); + let data_dir = temp_dir.path().display().to_string(); + + Self::spawn_spacetime_start(&[ + "start", + "--data-dir", + &data_dir, + ]) + } + + fn spawn_spacetime_start(extra_args: &[&str]) -> Self { + let port = find_free_port(); + let addr: SocketAddr = format!("127.0.0.1:{port}").parse().unwrap(); + let address = addr.to_string(); + let host_url = format!("http://{}", addr); + + // Workspace root for `cargo run -p ...` + let workspace_dir = env!("CARGO_MANIFEST_DIR"); + + Self::build_prereqs(workspace_dir); + + let mut cargo_args = vec![ + "run", + "-p", "spacetimedb-cli", + "--", + ]; + + cargo_args.extend(extra_args); + cargo_args.extend(["--listen-addr", &address]); + + let (child, logs) = Self::spawn_child(workspace_dir, &cargo_args); + + let guard = SpacetimeDbGuard { child, host_url, logs }; + guard.wait_until_http_ready(Duration::from_secs(10)); + guard + } + + // Ensure standalone is built before we start, if that’s needed. + // This is best-effort and usually a no-op when already built. + // Also build the CLI before running it to avoid that being included in the + // timeout for readiness. + fn build_prereqs(workspace_dir: &str) { + let targets = ["spacetimedb-standalone", "spacetimedb-cli"]; + + for pkg in targets { + let _ = Command::new("cargo") + .args(["build", "-p", pkg]) + .current_dir(workspace_dir) + .status() + .unwrap_or_else(|_| panic!("failed to build {}", pkg)); + } + } + + fn spawn_child(workspace_dir: &str, args: &[&str]) -> (Child, Arc>) { + let mut child = Command::new("cargo") + .args(args) + .current_dir(workspace_dir) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("failed to spawn spacetimedb-cli"); + + let logs = Arc::new(Mutex::new(String::new())); + + // Attach stdout logger + if let Some(stdout) = child.stdout.take() { + let logs_clone = logs.clone(); + thread::spawn(move || { + let reader = BufReader::new(stdout); + for line in reader.lines().map_while(Result::ok) { + let mut buf = logs_clone.lock().unwrap(); + buf.push_str("[STDOUT] "); + buf.push_str(&line); + buf.push('\n'); + } + }); + } + + // Attach stderr logger + if let Some(stderr) = child.stderr.take() { + let logs_clone = logs.clone(); + thread::spawn(move || { + let reader = BufReader::new(stderr); + for line in reader.lines().map_while(Result::ok) { + let mut buf = logs_clone.lock().unwrap(); + buf.push_str("[STDERR] "); + buf.push_str(&line); + buf.push('\n'); + } + }); + } + + (child, logs) + } + + fn wait_until_http_ready(&self, timeout: Duration) { + let client = Client::new(); + let deadline = Instant::now() + timeout; + + while Instant::now() < deadline { + let url = format!("{}/v1/ping", self.host_url); + + if let Ok(resp) = client.get(&url).send() { + if resp.status().is_success() { + return; // Fully ready! + } + } + + sleep(Duration::from_millis(50)); + } + panic!("Timed out waiting for SpacetimeDB HTTP /v1/ping at {}", self.host_url); + } +} + +impl Drop for SpacetimeDbGuard { + fn drop(&mut self) { + // Best-effort cleanup. + let _ = self.child.kill(); + let _ = self.child.wait(); + + // Only print logs if the test is currently panicking + if std::thread::panicking() { + if let Ok(logs) = self.logs.lock() { + eprintln!("\n===== SpacetimeDB child logs (only on failure) =====\n{}\n====================================================", *logs); + } + } + } +} diff --git a/crates/testing/src/modules.rs b/crates/testing/src/modules.rs index bd08d5d6555..2f12b1d690e 100644 --- a/crates/testing/src/modules.rs +++ b/crates/testing/src/modules.rs @@ -111,6 +111,7 @@ impl CompiledModule { &module_path(name), Some(PathBuf::from("src")).as_deref(), mode == CompilationMode::Debug, + None, ) .unwrap(); Self { diff --git a/modules/module-test/Cargo.toml b/modules/module-test/Cargo.toml index 14a2e984f66..ede1920648f 100644 --- a/modules/module-test/Cargo.toml +++ b/modules/module-test/Cargo.toml @@ -5,6 +5,9 @@ edition.workspace = true license-file = "LICENSE" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html\ +[features] +test-add-column = [] +test-remove-table = [] [lib] crate-type = ["cdylib"] diff --git a/modules/module-test/src/lib.rs b/modules/module-test/src/lib.rs index c745cd18457..fc12b219186 100644 --- a/modules/module-test/src/lib.rs +++ b/modules/module-test/src/lib.rs @@ -14,6 +14,7 @@ pub type TestAlias = TestA; // TABLE DEFINITIONS // ───────────────────────────────────────────────────────────────────────────── +#[cfg(feature = "test-add-column")] #[spacetimedb::table(name = person, public, index(name = age, btree(columns = [age])))] pub struct Person { #[primary_key] @@ -21,6 +22,24 @@ pub struct Person { id: u32, name: String, age: u8, + #[default(false)] + edited: bool, +} + +#[cfg(not(feature = "test-add-column"))] +#[spacetimedb::table(name = person, public, index(name = age, btree(columns = [age])))] +pub struct Person { + #[primary_key] + #[auto_inc] + id: u32, + name: String, + age: u8, +} + +#[cfg(not(feature = "test-remove-table"))] +#[spacetimedb::table(name = table_to_remove)] +pub struct RemoveTable { + pub id: u32, } #[spacetimedb::table(name = test_a, index(name = foo, btree(columns = [x])))] @@ -214,6 +233,9 @@ pub fn repeating_test(ctx: &ReducerContext, arg: RepeatingTestArg) { #[spacetimedb::reducer] pub fn add(ctx: &ReducerContext, name: String, age: u8) { + #[cfg(feature = "test-add-column")] + ctx.db.person().insert(Person { id: 0, name, age, edited: false }); + #[cfg(not(feature = "test-add-column"))] ctx.db.person().insert(Person { id: 0, name, age }); } From 3c994305d9aca1a8d774384eeca10381bde8ea8a Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Sat, 22 Nov 2025 22:28:20 -0500 Subject: [PATCH 03/20] cargo fmt --- crates/cli/src/tasks/rust.rs | 11 +++++++---- crates/cli/tests/publish.rs | 35 ++++++++++++++++++++++++++++++++++ crates/cli/tests/server.rs | 14 +++++--------- crates/cli/tests/util.rs | 22 +++++++++------------ modules/module-test/src/lib.rs | 11 ++++++++--- 5 files changed, 64 insertions(+), 29 deletions(-) diff --git a/crates/cli/src/tasks/rust.rs b/crates/cli/src/tasks/rust.rs index 9b6e52871b3..c5354fdae75 100644 --- a/crates/cli/src/tasks/rust.rs +++ b/crates/cli/src/tasks/rust.rs @@ -23,7 +23,12 @@ fn cargo_cmd(subcommand: &str, build_debug: bool, args: &[&str]) -> duct::Expres ) } -pub(crate) fn build_rust(project_path: &Path, features: Option<&std::ffi::OsString>, lint_dir: Option<&Path>, build_debug: bool) -> anyhow::Result { +pub(crate) fn build_rust( + project_path: &Path, + features: Option<&std::ffi::OsString>, + lint_dir: Option<&Path>, + build_debug: bool, +) -> anyhow::Result { // Make sure that we have the wasm target installed if !has_wasm32_target() { if has_rust_up() { @@ -85,9 +90,7 @@ pub(crate) fn build_rust(project_path: &Path, features: Option<&std::ffi::OsStri // Convert Vec to Vec<&str> let args_str: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let reader = cargo_cmd("build", build_debug, &args_str) - .dir(project_path) - .reader()?; + let reader = cargo_cmd("build", build_debug, &args_str).dir(project_path).reader()?; let mut artifact = None; for message in Message::parse_stream(io::BufReader::new(reader)) { diff --git a/crates/cli/tests/publish.rs b/crates/cli/tests/publish.rs index a57d9d5bf15..36cde30f14d 100644 --- a/crates/cli/tests/publish.rs +++ b/crates/cli/tests/publish.rs @@ -162,3 +162,38 @@ fn cli_can_publish_breaking_change_with_on_conflict_flag() { .assert() .success(); } + +#[test] +fn cli_can_publish_no_conflict_does_not_delete_data() { + let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); + + // Workspace root for `cargo run -p ...` + let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; + let dir = workspace_dir.join("modules").join("module-test"); + + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "publish", + "--server", + &spacetime.host_url.to_string(), + "no-conflict-test", + ]) + .current_dir(dir.clone()) + .assert() + .success(); + + // Can republish without conflict even with --on-conflict=delete-data flag + let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); + cmd.args([ + "publish", + "--server", + &spacetime.host_url.to_string(), + "--delete-data=on-conflict", + // NOTE: deleting data requires --yes, + // so not providing it here ensures that no data deletion is attempted. + "no-conflict-test", + ]) + .current_dir(dir) + .assert() + .success(); +} diff --git a/crates/cli/tests/server.rs b/crates/cli/tests/server.rs index 75bb44589a7..3e953f174d0 100644 --- a/crates/cli/tests/server.rs +++ b/crates/cli/tests/server.rs @@ -1,17 +1,13 @@ mod util; -use assert_cmd::cargo::cargo_bin_cmd; use crate::util::SpacetimeDbGuard; +use assert_cmd::cargo::cargo_bin_cmd; #[test] fn cli_can_ping_spacetimedb_on_disk() { let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "server", - "ping", - &spacetime.host_url.to_string(), - ]) - .assert() - .success(); -} \ No newline at end of file + cmd.args(["server", "ping", &spacetime.host_url.to_string()]) + .assert() + .success(); +} diff --git a/crates/cli/tests/util.rs b/crates/cli/tests/util.rs index 3f9d7a93a49..8bc1b26aeaf 100644 --- a/crates/cli/tests/util.rs +++ b/crates/cli/tests/util.rs @@ -1,6 +1,11 @@ - use std::{ - env, io::{BufReader, BufRead}, net::SocketAddr, process::{Child, Command, Stdio}, sync::{Arc, Mutex}, thread::{self, sleep}, time::{Duration, Instant} + env, + io::{BufRead, BufReader}, + net::SocketAddr, + process::{Child, Command, Stdio}, + sync::{Arc, Mutex}, + thread::{self, sleep}, + time::{Duration, Instant}, }; use reqwest::blocking::Client; @@ -16,18 +21,13 @@ pub struct SpacetimeDbGuard { } impl SpacetimeDbGuard { - /// Start `spacetimedb` in a temporary data directory via: /// cargo run -p spacetimedb-cli -- start --data-dir --listen-addr pub fn spawn_in_temp_data_dir() -> Self { let temp_dir = tempfile::tempdir().expect("failed to create temp dir"); let data_dir = temp_dir.path().display().to_string(); - Self::spawn_spacetime_start(&[ - "start", - "--data-dir", - &data_dir, - ]) + Self::spawn_spacetime_start(&["start", "--data-dir", &data_dir]) } fn spawn_spacetime_start(extra_args: &[&str]) -> Self { @@ -41,11 +41,7 @@ impl SpacetimeDbGuard { Self::build_prereqs(workspace_dir); - let mut cargo_args = vec![ - "run", - "-p", "spacetimedb-cli", - "--", - ]; + let mut cargo_args = vec!["run", "-p", "spacetimedb-cli", "--"]; cargo_args.extend(extra_args); cargo_args.extend(["--listen-addr", &address]); diff --git a/modules/module-test/src/lib.rs b/modules/module-test/src/lib.rs index fc12b219186..a5b59720cfd 100644 --- a/modules/module-test/src/lib.rs +++ b/modules/module-test/src/lib.rs @@ -233,9 +233,14 @@ pub fn repeating_test(ctx: &ReducerContext, arg: RepeatingTestArg) { #[spacetimedb::reducer] pub fn add(ctx: &ReducerContext, name: String, age: u8) { - #[cfg(feature = "test-add-column")] - ctx.db.person().insert(Person { id: 0, name, age, edited: false }); - #[cfg(not(feature = "test-add-column"))] + #[cfg(feature = "test-add-column")] + ctx.db.person().insert(Person { + id: 0, + name, + age, + edited: false, + }); + #[cfg(not(feature = "test-add-column"))] ctx.db.person().insert(Person { id: 0, name, age }); } From ed4f330ccb4ebbadc44002646443cc5f5b634e66 Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Sat, 22 Nov 2025 22:41:06 -0500 Subject: [PATCH 04/20] Snap tests --- .../snapshots/codegen__codegen_csharp.snap | 61 +++++++- .../snapshots/codegen__codegen_rust.snap | 135 ++++++++++++++++++ .../codegen__codegen_typescript.snap | 47 ++++++ 3 files changed, 242 insertions(+), 1 deletion(-) diff --git a/crates/codegen/tests/snapshots/codegen__codegen_csharp.snap b/crates/codegen/tests/snapshots/codegen__codegen_csharp.snap index 813b4e2a1e6..f5c0f73934a 100644 --- a/crates/codegen/tests/snapshots/codegen__codegen_csharp.snap +++ b/crates/codegen/tests/snapshots/codegen__codegen_csharp.snap @@ -1,6 +1,5 @@ --- source: crates/codegen/tests/codegen.rs -assertion_line: 37 expression: outfiles --- "Procedures/GetMySchemaViaHttp.g.cs" = ''' @@ -1296,6 +1295,7 @@ namespace SpacetimeDB AddTable(Points = new(conn)); AddTable(PrivateTable = new(conn)); AddTable(RepeatingTestArg = new(conn)); + AddTable(TableToRemove = new(conn)); AddTable(TestA = new(conn)); AddTable(TestD = new(conn)); AddTable(TestE = new(conn)); @@ -2301,6 +2301,35 @@ namespace SpacetimeDB } } ''' +"Tables/TableToRemove.g.cs" = ''' +// THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE +// WILL NOT BE SAVED. MODIFY TABLES IN YOUR MODULE SOURCE CODE INSTEAD. + +#nullable enable + +using System; +using SpacetimeDB.BSATN; +using SpacetimeDB.ClientApi; +using System.Collections.Generic; +using System.Runtime.Serialization; + +namespace SpacetimeDB +{ + public sealed partial class RemoteTables + { + public sealed class TableToRemoveHandle : RemoteTableHandle + { + protected override string RemoteTableName => "table_to_remove"; + + internal TableToRemoveHandle(DbConnection conn) : base(conn) + { + } + } + + public readonly TableToRemoveHandle TableToRemove; + } +} +''' "Tables/TestA.g.cs" = ''' // THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE // WILL NOT BE SAVED. MODIFY TABLES IN YOUR MODULE SOURCE CODE INSTEAD. @@ -2755,6 +2784,36 @@ namespace SpacetimeDB } } ''' +"Types/RemoveTable.g.cs" = ''' +// THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE +// WILL NOT BE SAVED. MODIFY TABLES IN YOUR MODULE SOURCE CODE INSTEAD. + +#nullable enable + +using System; +using System.Collections.Generic; +using System.Runtime.Serialization; + +namespace SpacetimeDB +{ + [SpacetimeDB.Type] + [DataContract] + public sealed partial class RemoveTable + { + [DataMember(Name = "id")] + public uint Id; + + public RemoveTable(uint Id) + { + this.Id = Id; + } + + public RemoveTable() + { + } + } +} +''' "Types/RepeatingTestArg.g.cs" = ''' // THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE // WILL NOT BE SAVED. MODIFY TABLES IN YOUR MODULE SOURCE CODE INSTEAD. diff --git a/crates/codegen/tests/snapshots/codegen__codegen_rust.snap b/crates/codegen/tests/snapshots/codegen__codegen_rust.snap index 95c9ce2fac1..1f5ba6e7414 100644 --- a/crates/codegen/tests/snapshots/codegen__codegen_rust.snap +++ b/crates/codegen/tests/snapshots/codegen__codegen_rust.snap @@ -1441,6 +1441,7 @@ pub mod pk_multi_identity_type; pub mod player_type; pub mod point_type; pub mod private_table_type; +pub mod remove_table_type; pub mod repeating_test_arg_type; pub mod test_a_type; pub mod test_b_type; @@ -1471,6 +1472,7 @@ pub mod player_table; pub mod points_table; pub mod private_table_table; pub mod repeating_test_arg_table; +pub mod table_to_remove_table; pub mod test_a_table; pub mod test_d_table; pub mod test_e_table; @@ -1489,6 +1491,7 @@ pub use pk_multi_identity_type::PkMultiIdentity; pub use player_type::Player; pub use point_type::Point; pub use private_table_type::PrivateTable; +pub use remove_table_type::RemoveTable; pub use repeating_test_arg_type::RepeatingTestArg; pub use test_a_type::TestA; pub use test_b_type::TestB; @@ -1506,6 +1509,7 @@ pub use player_table::*; pub use points_table::*; pub use private_table_table::*; pub use repeating_test_arg_table::*; +pub use table_to_remove_table::*; pub use test_a_table::*; pub use test_d_table::*; pub use test_e_table::*; @@ -1635,6 +1639,7 @@ pub struct DbUpdate { points: __sdk::TableUpdate, private_table: __sdk::TableUpdate, repeating_test_arg: __sdk::TableUpdate, + table_to_remove: __sdk::TableUpdate, test_a: __sdk::TableUpdate, test_d: __sdk::TableUpdate, test_e: __sdk::TableUpdate, @@ -1658,6 +1663,7 @@ impl TryFrom<__ws::DatabaseUpdate<__ws::BsatnFormat>> for DbUpdate { "points" => db_update.points.append(points_table::parse_table_update(table_update)?), "private_table" => db_update.private_table.append(private_table_table::parse_table_update(table_update)?), "repeating_test_arg" => db_update.repeating_test_arg.append(repeating_test_arg_table::parse_table_update(table_update)?), + "table_to_remove" => db_update.table_to_remove.append(table_to_remove_table::parse_table_update(table_update)?), "test_a" => db_update.test_a.append(test_a_table::parse_table_update(table_update)?), "test_d" => db_update.test_d.append(test_d_table::parse_table_update(table_update)?), "test_e" => db_update.test_e.append(test_e_table::parse_table_update(table_update)?), @@ -1692,6 +1698,7 @@ impl __sdk::DbUpdate for DbUpdate { diff.points = cache.apply_diff_to_table::("points", &self.points); diff.private_table = cache.apply_diff_to_table::("private_table", &self.private_table); diff.repeating_test_arg = cache.apply_diff_to_table::("repeating_test_arg", &self.repeating_test_arg).with_updates_by_pk(|row| &row.scheduled_id); + diff.table_to_remove = cache.apply_diff_to_table::("table_to_remove", &self.table_to_remove); diff.test_a = cache.apply_diff_to_table::("test_a", &self.test_a); diff.test_d = cache.apply_diff_to_table::("test_d", &self.test_d); diff.test_e = cache.apply_diff_to_table::("test_e", &self.test_e).with_updates_by_pk(|row| &row.id); @@ -1715,6 +1722,7 @@ pub struct AppliedDiff<'r> { points: __sdk::TableAppliedDiff<'r, Point>, private_table: __sdk::TableAppliedDiff<'r, PrivateTable>, repeating_test_arg: __sdk::TableAppliedDiff<'r, RepeatingTestArg>, + table_to_remove: __sdk::TableAppliedDiff<'r, RemoveTable>, test_a: __sdk::TableAppliedDiff<'r, TestA>, test_d: __sdk::TableAppliedDiff<'r, TestD>, test_e: __sdk::TableAppliedDiff<'r, TestE>, @@ -1738,6 +1746,7 @@ impl<'r> __sdk::AppliedDiff<'r> for AppliedDiff<'r> { callbacks.invoke_table_row_callbacks::("points", &self.points, event); callbacks.invoke_table_row_callbacks::("private_table", &self.private_table, event); callbacks.invoke_table_row_callbacks::("repeating_test_arg", &self.repeating_test_arg, event); + callbacks.invoke_table_row_callbacks::("table_to_remove", &self.table_to_remove, event); callbacks.invoke_table_row_callbacks::("test_a", &self.test_a, event); callbacks.invoke_table_row_callbacks::("test_d", &self.test_d, event); callbacks.invoke_table_row_callbacks::("test_e", &self.test_e, event); @@ -2467,6 +2476,7 @@ fn register_tables(client_cache: &mut __sdk::ClientCache) { points_table::register_table(client_cache); private_table_table::register_table(client_cache); repeating_test_arg_table::register_table(client_cache); + table_to_remove_table::register_table(client_cache); test_a_table::register_table(client_cache); test_d_table::register_table(client_cache); test_e_table::register_table(client_cache); @@ -3605,6 +3615,31 @@ impl set_flags_for_query_private for super::SetReducerFlags { } } +''' +"remove_table_type.rs" = ''' +// THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE +// WILL NOT BE SAVED. MODIFY TABLES IN YOUR MODULE SOURCE CODE INSTEAD. + +#![allow(unused, clippy::all)] +use spacetimedb_sdk::__codegen::{ + self as __sdk, + __lib, + __sats, + __ws, +}; + + +#[derive(__lib::ser::Serialize, __lib::de::Deserialize, Clone, PartialEq, Debug)] +#[sats(crate = __lib)] +pub struct RemoveTable { + pub id: u32, +} + + +impl __sdk::InModule for RemoveTable { + type Module = super::RemoteModule; +} + ''' "repeating_test_arg_table.rs" = ''' // THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE @@ -4107,6 +4142,106 @@ impl sleep_one_second for super::RemoteProcedures { } } +''' +"table_to_remove_table.rs" = ''' +// THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE +// WILL NOT BE SAVED. MODIFY TABLES IN YOUR MODULE SOURCE CODE INSTEAD. + +#![allow(unused, clippy::all)] +use spacetimedb_sdk::__codegen::{ + self as __sdk, + __lib, + __sats, + __ws, +}; +use super::remove_table_type::RemoveTable; + +/// Table handle for the table `table_to_remove`. +/// +/// Obtain a handle from the [`TableToRemoveTableAccess::table_to_remove`] method on [`super::RemoteTables`], +/// like `ctx.db.table_to_remove()`. +/// +/// Users are encouraged not to explicitly reference this type, +/// but to directly chain method calls, +/// like `ctx.db.table_to_remove().on_insert(...)`. +pub struct TableToRemoveTableHandle<'ctx> { + imp: __sdk::TableHandle, + ctx: std::marker::PhantomData<&'ctx super::RemoteTables>, +} + +#[allow(non_camel_case_types)] +/// Extension trait for access to the table `table_to_remove`. +/// +/// Implemented for [`super::RemoteTables`]. +pub trait TableToRemoveTableAccess { + #[allow(non_snake_case)] + /// Obtain a [`TableToRemoveTableHandle`], which mediates access to the table `table_to_remove`. + fn table_to_remove(&self) -> TableToRemoveTableHandle<'_>; +} + +impl TableToRemoveTableAccess for super::RemoteTables { + fn table_to_remove(&self) -> TableToRemoveTableHandle<'_> { + TableToRemoveTableHandle { + imp: self.imp.get_table::("table_to_remove"), + ctx: std::marker::PhantomData, + } + } +} + +pub struct TableToRemoveInsertCallbackId(__sdk::CallbackId); +pub struct TableToRemoveDeleteCallbackId(__sdk::CallbackId); + +impl<'ctx> __sdk::Table for TableToRemoveTableHandle<'ctx> { + type Row = RemoveTable; + type EventContext = super::EventContext; + + fn count(&self) -> u64 { self.imp.count() } + fn iter(&self) -> impl Iterator + '_ { self.imp.iter() } + + type InsertCallbackId = TableToRemoveInsertCallbackId; + + fn on_insert( + &self, + callback: impl FnMut(&Self::EventContext, &Self::Row) + Send + 'static, + ) -> TableToRemoveInsertCallbackId { + TableToRemoveInsertCallbackId(self.imp.on_insert(Box::new(callback))) + } + + fn remove_on_insert(&self, callback: TableToRemoveInsertCallbackId) { + self.imp.remove_on_insert(callback.0) + } + + type DeleteCallbackId = TableToRemoveDeleteCallbackId; + + fn on_delete( + &self, + callback: impl FnMut(&Self::EventContext, &Self::Row) + Send + 'static, + ) -> TableToRemoveDeleteCallbackId { + TableToRemoveDeleteCallbackId(self.imp.on_delete(Box::new(callback))) + } + + fn remove_on_delete(&self, callback: TableToRemoveDeleteCallbackId) { + self.imp.remove_on_delete(callback.0) + } +} + +#[doc(hidden)] +pub(super) fn register_table(client_cache: &mut __sdk::ClientCache) { + + let _table = client_cache.get_or_make_table::("table_to_remove"); +} + +#[doc(hidden)] +pub(super) fn parse_table_update( + raw_updates: __ws::TableUpdate<__ws::BsatnFormat>, +) -> __sdk::Result<__sdk::TableUpdate> { + __sdk::TableUpdate::parse_table_update(raw_updates).map_err(|e| { + __sdk::InternalError::failed_parse( + "TableUpdate", + "TableUpdate", + ).with_cause(e).into() + }) +} ''' "test_a_table.rs" = ''' // THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE diff --git a/crates/codegen/tests/snapshots/codegen__codegen_typescript.snap b/crates/codegen/tests/snapshots/codegen__codegen_typescript.snap index 404c7e4fe82..dc3dbafacf6 100644 --- a/crates/codegen/tests/snapshots/codegen__codegen_typescript.snap +++ b/crates/codegen/tests/snapshots/codegen__codegen_typescript.snap @@ -282,6 +282,8 @@ import PrivateTableRow from "./private_table_table"; export { PrivateTableRow }; import RepeatingTestArgRow from "./repeating_test_arg_table"; export { RepeatingTestArgRow }; +import TableToRemoveRow from "./table_to_remove_table"; +export { TableToRemoveRow }; import TestARow from "./test_a_table"; export { TestARow }; import TestDRow from "./test_d_table"; @@ -308,6 +310,8 @@ import Point from "./point_type"; export { Point }; import PrivateTable from "./private_table_type"; export { PrivateTable }; +import RemoveTable from "./remove_table_type"; +export { RemoveTable }; import RepeatingTestArg from "./repeating_test_arg_type"; export { RepeatingTestArg }; import TestA from "./test_a_type"; @@ -430,6 +434,13 @@ const tablesSchema = __schema( { name: 'repeating_test_arg_scheduled_id_key', constraint: 'unique', columns: ['scheduled_id'] }, ], }, RepeatingTestArgRow), + __table({ + name: 'table_to_remove', + indexes: [ + ], + constraints: [ + ], + }, RemoveTableRow), __table({ name: 'test_a', indexes: [ @@ -852,6 +863,25 @@ import { } from "spacetimedb"; export default {}; +''' +"remove_table_type.ts" = ''' +// THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE +// WILL NOT BE SAVED. MODIFY TABLES IN YOUR MODULE SOURCE CODE INSTEAD. + +/* eslint-disable */ +/* tslint:disable */ +import { + TypeBuilder as __TypeBuilder, + t as __t, + type AlgebraicTypeType as __AlgebraicTypeType, + type Infer as __Infer, +} from "spacetimedb"; + +export default __t.object("RemoveTable", { + id: __t.u32(), +}); + + ''' "repeating_test_arg_table.ts" = ''' // THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE @@ -931,6 +961,23 @@ import { export default {}; ''' "sleep_one_second_procedure.ts" = '' +"table_to_remove_table.ts" = ''' +// THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE +// WILL NOT BE SAVED. MODIFY TABLES IN YOUR MODULE SOURCE CODE INSTEAD. + +/* eslint-disable */ +/* tslint:disable */ +import { + TypeBuilder as __TypeBuilder, + t as __t, + type AlgebraicTypeType as __AlgebraicTypeType, + type Infer as __Infer, +} from "spacetimedb"; + +export default __t.row({ + id: __t.u32(), +}); +''' "test_a_table.ts" = ''' // THIS FILE IS AUTOMATICALLY GENERATED BY SPACETIMEDB. EDITS TO THIS FILE // WILL NOT BE SAVED. MODIFY TABLES IN YOUR MODULE SOURCE CODE INSTEAD. From 8af9cf090d7e3a3ae808ac0c56dfec2e537963f4 Mon Sep 17 00:00:00 2001 From: Tyler Cloutier Date: Sun, 23 Nov 2025 23:35:10 -0500 Subject: [PATCH 05/20] added table_to_remove to the other module-tests --- modules/module-test-cs/Lib.cs | 6 ++++++ modules/module-test-ts/src/index.ts | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/modules/module-test-cs/Lib.cs b/modules/module-test-cs/Lib.cs index 1889f75bf2e..a8d7d7a05ba 100644 --- a/modules/module-test-cs/Lib.cs +++ b/modules/module-test-cs/Lib.cs @@ -178,6 +178,12 @@ public Player() public string name; } +[Table(Name = "table_to_remove")] +public partial struct TableToRemove +{ + public uint id; +} + // ───────────────────────────────────────────────────────────────────────────── // SUPPORT TYPES // ───────────────────────────────────────────────────────────────────────────── diff --git a/modules/module-test-ts/src/index.ts b/modules/module-test-ts/src/index.ts index 659bde643eb..19f74771916 100644 --- a/modules/module-test-ts/src/index.ts +++ b/modules/module-test-ts/src/index.ts @@ -211,7 +211,8 @@ const spacetimedb = schema( // Two tables with the same row type: player and logged_out_player table({ name: 'player', public: true }, playerLikeRow), - table({ name: 'logged_out_player', public: true }, playerLikeRow) + table({ name: 'logged_out_player', public: true }, playerLikeRow), + table({ name: 'table_to_remove' }, { id: t.u32() }) ); // ───────────────────────────────────────────────────────────────────────────── From 239f3668a79e998d279c1b858387bf9d77fd4135 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 25 Nov 2025 12:27:15 -0800 Subject: [PATCH 06/20] [tyler/fix-on-conflict]: clear env vars to improve build time --- crates/cli/tests/util.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/cli/tests/util.rs b/crates/cli/tests/util.rs index 8bc1b26aeaf..e9d7c48e16a 100644 --- a/crates/cli/tests/util.rs +++ b/crates/cli/tests/util.rs @@ -20,6 +20,16 @@ pub struct SpacetimeDbGuard { pub logs: Arc>, } +// Remove all Cargo-provided env vars. These are set by the fact that we're running in a cargo command (e.g. `cargo test`). +// We don't want to inherit any of these to a child cargo process, because it causes unnecessary rebuilds. +fn unset_cargo_env_vars() -> () { + for (key, _) in std::env::vars() { + if key.starts_with("CARGO_") && key != "CARGO_TARGET_DIR" { + std::env::remove_var(key); + } + } +} + impl SpacetimeDbGuard { /// Start `spacetimedb` in a temporary data directory via: /// cargo run -p spacetimedb-cli -- start --data-dir --listen-addr @@ -41,6 +51,7 @@ impl SpacetimeDbGuard { Self::build_prereqs(workspace_dir); + unset_cargo_env_vars(); let mut cargo_args = vec!["run", "-p", "spacetimedb-cli", "--"]; cargo_args.extend(extra_args); @@ -60,6 +71,7 @@ impl SpacetimeDbGuard { fn build_prereqs(workspace_dir: &str) { let targets = ["spacetimedb-standalone", "spacetimedb-cli"]; + unset_cargo_env_vars(); for pkg in targets { let _ = Command::new("cargo") .args(["build", "-p", pkg]) From b27fe6a83bc137ef329305098dcc226156629f8f Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 25 Nov 2025 12:38:03 -0800 Subject: [PATCH 07/20] [tyler/fix-on-conflict]: refactoring --- crates/cli/tests/publish.rs | 188 +++++++++--------------------------- 1 file changed, 47 insertions(+), 141 deletions(-) diff --git a/crates/cli/tests/publish.rs b/crates/cli/tests/publish.rs index 36cde30f14d..3170d07dda6 100644 --- a/crates/cli/tests/publish.rs +++ b/crates/cli/tests/publish.rs @@ -26,174 +26,80 @@ fn cli_can_publish_spacetimedb_on_disk() { .success(); } -#[test] -fn cli_can_publish_with_automigration_change() { +// TODO: Somewhere we should test that --delete-data actually deletes the data in all cases +fn migration_test(module_name: &str, republish_args: &[&str], expect_success: bool) -> () { let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); - // Workspace root for `cargo run -p ...` let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; let dir = workspace_dir.join("modules").join("module-test"); let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "publish", - "--server", - &spacetime.host_url.to_string(), - "automigration-test", - ]) - .current_dir(dir.clone()) - .assert() - .success(); + cmd.args(["publish", module_name, "--server", &spacetime.host_url.to_string()]) + .current_dir(dir.clone()) + .assert() + .success(); - // Can republish with automigration change let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "publish", - "--build-options=--features test-add-column", - "--server", - &spacetime.host_url.to_string(), - "--yes-break-clients", + cmd.args(["publish", module_name, "--server", &spacetime.host_url.to_string()]) + .args(republish_args) + .current_dir(dir); + + if expect_success { + cmd.assert().success(); + } else { + cmd.assert().failure(); + } +} + +#[test] +fn cli_can_publish_with_automigration_change() { + migration_test( "automigration-test", - ]) - .current_dir(dir) - .assert() - .success(); + &["--build-options=--features test-add-column", "--yes-break-clients"], + true, + ); } #[test] fn cli_cannot_publish_breaking_change_without_flag() { - let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); - - // Workspace root for `cargo run -p ...` - let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; - let dir = workspace_dir.join("modules").join("module-test"); - - let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "publish", - "--server", - &spacetime.host_url.to_string(), + migration_test( "breaking-change-test", - ]) - .current_dir(dir.clone()) - .assert() - .success(); - - // Cannot republish with breaking change without flag - let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "publish", - "--build-options=--features test-remove-table", - "--server", - &spacetime.host_url.to_string(), - "breaking-change-test", - ]) - .current_dir(dir) - .assert() - .failure(); + &["--build-options=--features test-remove-table"], + false, + ); } #[test] fn cli_can_publish_breaking_change_with_delete_data_flag() { - let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); - - // Workspace root for `cargo run -p ...` - let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; - let dir = workspace_dir.join("modules").join("module-test"); - - let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "publish", - "--server", - &spacetime.host_url.to_string(), + migration_test( "breaking-change-delete-data-test", - ]) - .current_dir(dir.clone()) - .assert() - .success(); - - // Can republish with breaking change with --delete-data flag - let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "publish", - "--build-options=--features test-remove-table", - "--server", - &spacetime.host_url.to_string(), - "--delete-data", - "--yes", - "breaking-change-delete-data-test", - ]) - .current_dir(dir) - .assert() - .success(); + &["--build-options=--features test-remove-table", "--delete-data", "--yes"], + true, + ); } #[test] fn cli_can_publish_breaking_change_with_on_conflict_flag() { - let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); - - // Workspace root for `cargo run -p ...` - let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; - let dir = workspace_dir.join("modules").join("module-test"); - - let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "publish", - "--server", - &spacetime.host_url.to_string(), + migration_test( "breaking-change-on-conflict-test", - ]) - .current_dir(dir.clone()) - .assert() - .success(); - - // Can republish with breaking change with --on-conflict=delete-data flag - let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "publish", - "--build-options=--features test-remove-table", - "--server", - &spacetime.host_url.to_string(), - "--delete-data=on-conflict", - "--yes", - "breaking-change-on-conflict-test", - ]) - .current_dir(dir) - .assert() - .success(); + &[ + "--build-options=--features test-remove-table", + "--delete-data=on-conflict", + "--yes", + ], + true, + ); } #[test] fn cli_can_publish_no_conflict_does_not_delete_data() { - let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); - - // Workspace root for `cargo run -p ...` - let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; - let dir = workspace_dir.join("modules").join("module-test"); - - let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "publish", - "--server", - &spacetime.host_url.to_string(), - "no-conflict-test", - ]) - .current_dir(dir.clone()) - .assert() - .success(); - - // Can republish without conflict even with --on-conflict=delete-data flag - let mut cmd = cargo_bin_cmd!("spacetimedb-cli"); - cmd.args([ - "publish", - "--server", - &spacetime.host_url.to_string(), - "--delete-data=on-conflict", - // NOTE: deleting data requires --yes, - // so not providing it here ensures that no data deletion is attempted. + migration_test( "no-conflict-test", - ]) - .current_dir(dir) - .assert() - .success(); + &[ + "--delete-data=on-conflict", + // NOTE: deleting data requires --yes, + // so not providing it here ensures that no data deletion is attempted. + ], + true, + ); } From a1f0cb22b41662199799809d5096352ef24987ad Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 25 Nov 2025 13:02:51 -0800 Subject: [PATCH 08/20] [tyler/fix-on-conflict]: more tests --- crates/cli/tests/publish.rs | 101 +++++++++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 14 deletions(-) diff --git a/crates/cli/tests/publish.rs b/crates/cli/tests/publish.rs index 3170d07dda6..052c9343bf9 100644 --- a/crates/cli/tests/publish.rs +++ b/crates/cli/tests/publish.rs @@ -26,7 +26,9 @@ fn cli_can_publish_spacetimedb_on_disk() { .success(); } -// TODO: Somewhere we should test that --delete-data actually deletes the data in all cases +// TODO: Somewhere we should test that data is actually deleted properly in all the expected cases, +// e.g. when providing --delete-data, or when there's a conflict and --delete-data=on-conflict is provided. + fn migration_test(module_name: &str, republish_args: &[&str], expect_success: bool) -> () { let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); @@ -51,6 +53,33 @@ fn migration_test(module_name: &str, republish_args: &[&str], expect_success: bo } } +#[test] +fn cli_can_publish_no_conflict_does_not_delete_data() { + migration_test( + "no-conflict-test", + &[ + "--delete-data=on-conflict", + // NOTE: deleting data requires --yes, + // so not providing it here ensures that no data deletion is attempted. + ], + true, + ); +} + +#[test] +fn cli_can_publish_no_conflict_with_delete_data_flag() { + migration_test("no-conflict-delete-data-test", &["--delete-data", "--yes"], true); +} + +#[test] +fn cli_can_publish_no_conflict_without_delete_data_flag() { + migration_test( + "no-conflict-test", + &[], // no --delete-data flag at all + true, + ); +} + #[test] fn cli_can_publish_with_automigration_change() { migration_test( @@ -61,44 +90,88 @@ fn cli_can_publish_with_automigration_change() { } #[test] -fn cli_cannot_publish_breaking_change_without_flag() { +fn cli_cannot_publish_automigration_change_without_yes_break_clients() { migration_test( - "breaking-change-test", - &["--build-options=--features test-remove-table"], + "automigration-test-no-break-flag", + &["--build-options=--features test-add-column"], false, ); } #[test] -fn cli_can_publish_breaking_change_with_delete_data_flag() { +fn cli_can_publish_automigration_change_with_on_conflict_and_yes_break_clients() { migration_test( - "breaking-change-delete-data-test", - &["--build-options=--features test-remove-table", "--delete-data", "--yes"], + "automigration-on-conflict-test", + &[ + "--build-options=--features test-add-column", + "--delete-data=on-conflict", + "--yes-break-clients", + ], true, ); } #[test] -fn cli_can_publish_breaking_change_with_on_conflict_flag() { +fn cli_cannot_publish_automigration_change_with_on_conflict_without_yes_break_clients() { migration_test( - "breaking-change-on-conflict-test", + "automigration-on-conflict-no-break-flag-test", &[ - "--build-options=--features test-remove-table", + "--build-options=--features test-add-column", "--delete-data=on-conflict", + ], + false, + ); +} + +#[test] +fn cli_can_publish_automigration_change_with_delete_data_always_without_yes_break_clients() { + migration_test( + "automigration-delete-data-test", + &["--build-options=--features test-add-column", "--delete-data", "--yes"], + true, + ); +} + +#[test] +fn cli_can_publish_automigration_change_with_delete_data_always_and_yes_break_clients() { + migration_test( + "automigration-delete-data-break-test", + &[ + "--build-options=--features test-add-column", + "--delete-data", "--yes", + "--yes-break-clients", ], true, ); } #[test] -fn cli_can_publish_no_conflict_does_not_delete_data() { +fn cli_cannot_publish_breaking_change_without_flag() { migration_test( - "no-conflict-test", + "breaking-change-test", + &["--build-options=--features test-remove-table"], + false, + ); +} + +#[test] +fn cli_can_publish_breaking_change_with_delete_data_flag() { + migration_test( + "breaking-change-delete-data-test", + &["--build-options=--features test-remove-table", "--delete-data", "--yes"], + true, + ); +} + +#[test] +fn cli_can_publish_breaking_change_with_on_conflict_flag() { + migration_test( + "breaking-change-on-conflict-test", &[ + "--build-options=--features test-remove-table", "--delete-data=on-conflict", - // NOTE: deleting data requires --yes, - // so not providing it here ensures that no data deletion is attempted. + "--yes", ], true, ); From 2c4e1eee8b1db15f94b20950d7993473843f09f3 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 25 Nov 2025 13:13:26 -0800 Subject: [PATCH 09/20] [tyler/fix-on-conflict]: clippy --- crates/cli/tests/publish.rs | 2 +- crates/cli/tests/util.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/cli/tests/publish.rs b/crates/cli/tests/publish.rs index 052c9343bf9..e4c837dfa07 100644 --- a/crates/cli/tests/publish.rs +++ b/crates/cli/tests/publish.rs @@ -29,7 +29,7 @@ fn cli_can_publish_spacetimedb_on_disk() { // TODO: Somewhere we should test that data is actually deleted properly in all the expected cases, // e.g. when providing --delete-data, or when there's a conflict and --delete-data=on-conflict is provided. -fn migration_test(module_name: &str, republish_args: &[&str], expect_success: bool) -> () { +fn migration_test(module_name: &str, republish_args: &[&str], expect_success: bool) { let spacetime = SpacetimeDbGuard::spawn_in_temp_data_dir(); let workspace_dir = cargo_metadata::MetadataCommand::new().exec().unwrap().workspace_root; diff --git a/crates/cli/tests/util.rs b/crates/cli/tests/util.rs index e9d7c48e16a..0195232137d 100644 --- a/crates/cli/tests/util.rs +++ b/crates/cli/tests/util.rs @@ -22,7 +22,7 @@ pub struct SpacetimeDbGuard { // Remove all Cargo-provided env vars. These are set by the fact that we're running in a cargo command (e.g. `cargo test`). // We don't want to inherit any of these to a child cargo process, because it causes unnecessary rebuilds. -fn unset_cargo_env_vars() -> () { +fn unset_cargo_env_vars() { for (key, _) in std::env::vars() { if key.starts_with("CARGO_") && key != "CARGO_TARGET_DIR" { std::env::remove_var(key); From 9953d9858e31cd2ebfc08b19c1d112c01a9cad14 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 25 Nov 2025 13:19:35 -0800 Subject: [PATCH 10/20] [tyler/fix-on-conflict]: review --- crates/cli/tests/publish.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/cli/tests/publish.rs b/crates/cli/tests/publish.rs index e4c837dfa07..f711064026f 100644 --- a/crates/cli/tests/publish.rs +++ b/crates/cli/tests/publish.rs @@ -58,9 +58,9 @@ fn cli_can_publish_no_conflict_does_not_delete_data() { migration_test( "no-conflict-test", &[ - "--delete-data=on-conflict", // NOTE: deleting data requires --yes, // so not providing it here ensures that no data deletion is attempted. + "--delete-data=on-conflict", ], true, ); @@ -73,11 +73,7 @@ fn cli_can_publish_no_conflict_with_delete_data_flag() { #[test] fn cli_can_publish_no_conflict_without_delete_data_flag() { - migration_test( - "no-conflict-test", - &[], // no --delete-data flag at all - true, - ); + migration_test("no-conflict-test", &[], true); } #[test] @@ -104,6 +100,8 @@ fn cli_can_publish_automigration_change_with_on_conflict_and_yes_break_clients() "automigration-on-conflict-test", &[ "--build-options=--features test-add-column", + // NOTE: deleting data requires --yes, + // so not providing it here ensures that no data deletion is attempted. "--delete-data=on-conflict", "--yes-break-clients", ], @@ -117,6 +115,8 @@ fn cli_cannot_publish_automigration_change_with_on_conflict_without_yes_break_cl "automigration-on-conflict-no-break-flag-test", &[ "--build-options=--features test-add-column", + // NOTE: deleting data requires --yes, + // so not providing it here ensures that no data deletion is attempted. "--delete-data=on-conflict", ], false, From 071a66ab23a5296cace023bf44514304ef96d845 Mon Sep 17 00:00:00 2001 From: Zeke Foppa <196249+bfops@users.noreply.github.com> Date: Tue, 25 Nov 2025 14:04:43 -0800 Subject: [PATCH 11/20] Update crates/cli/Cargo.toml Co-authored-by: Phoebe Goldman Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com> --- crates/cli/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 58891a78a26..e0d3663176c 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -90,7 +90,7 @@ fs_extra.workspace = true assert_cmd = "2" predicates = "3" portpicker = "0.1" -reqwest = { version = "0.12", features = ["blocking", "json"] } +reqwest = { workspace = true, features = ["blocking", "json"] } [target.'cfg(not(target_env = "msvc"))'.dependencies] tikv-jemallocator = { workspace = true } From 67c57a3d887ce28befc7ba8d0d4a3fb8657808c9 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 25 Nov 2025 15:02:12 -0800 Subject: [PATCH 12/20] [tyler/fix-on-conflict]: try fix? --- crates/cli/tests/util.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/crates/cli/tests/util.rs b/crates/cli/tests/util.rs index 0195232137d..182fb0672f1 100644 --- a/crates/cli/tests/util.rs +++ b/crates/cli/tests/util.rs @@ -20,12 +20,13 @@ pub struct SpacetimeDbGuard { pub logs: Arc>, } -// Remove all Cargo-provided env vars. These are set by the fact that we're running in a cargo command (e.g. `cargo test`). -// We don't want to inherit any of these to a child cargo process, because it causes unnecessary rebuilds. -fn unset_cargo_env_vars() { +// Remove all Cargo-provided env vars from a child process. These are set by the fact that we're running in a cargo +// command (e.g. `cargo test`). We don't want to inherit any of these to a child cargo process, because it causes +// unnecessary rebuilds. +fn unset_cargo_env_vars(cmd: &mut Command) { for (key, _) in std::env::vars() { if key.starts_with("CARGO_") && key != "CARGO_TARGET_DIR" { - std::env::remove_var(key); + cmd.env_remove(&key); } } } @@ -50,8 +51,6 @@ impl SpacetimeDbGuard { let workspace_dir = env!("CARGO_MANIFEST_DIR"); Self::build_prereqs(workspace_dir); - - unset_cargo_env_vars(); let mut cargo_args = vec!["run", "-p", "spacetimedb-cli", "--"]; cargo_args.extend(extra_args); @@ -71,9 +70,10 @@ impl SpacetimeDbGuard { fn build_prereqs(workspace_dir: &str) { let targets = ["spacetimedb-standalone", "spacetimedb-cli"]; - unset_cargo_env_vars(); for pkg in targets { - let _ = Command::new("cargo") + let mut cmd = Command::new("cargo"); + unset_cargo_env_vars(&mut cmd); + let _ = cmd .args(["build", "-p", pkg]) .current_dir(workspace_dir) .status() @@ -82,7 +82,9 @@ impl SpacetimeDbGuard { } fn spawn_child(workspace_dir: &str, args: &[&str]) -> (Child, Arc>) { - let mut child = Command::new("cargo") + let mut cmd = Command::new("cargo"); + unset_cargo_env_vars(&mut cmd); + let mut child = cmd .args(args) .current_dir(workspace_dir) .stdout(Stdio::piped()) From 3ed2f129a42dd2e1ab6d680a2a99ff1fbf2292bd Mon Sep 17 00:00:00 2001 From: John Detter <4099508+jdetter@users.noreply.github.com> Date: Tue, 25 Nov 2025 18:34:54 -0600 Subject: [PATCH 13/20] Remove alias --yes-break-clients --- crates/cli/src/subcommands/publish.rs | 1 - crates/cli/tests/publish.rs | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/cli/src/subcommands/publish.rs b/crates/cli/src/subcommands/publish.rs index 044344676b5..ad94a3f4d84 100644 --- a/crates/cli/src/subcommands/publish.rs +++ b/crates/cli/src/subcommands/publish.rs @@ -67,7 +67,6 @@ pub fn cli() -> clap::Command { .arg( Arg::new("break_clients") .long("break-clients") - .alias("yes-break-clients") .action(SetTrue) .help("Allow breaking changes when publishing to an existing database identity. This will force publish even if it will break existing clients, but will NOT force publish if it would cause deletion of any data in the database. See --yes and --delete-data for details.") ) diff --git a/crates/cli/tests/publish.rs b/crates/cli/tests/publish.rs index f711064026f..a4df1bf88ca 100644 --- a/crates/cli/tests/publish.rs +++ b/crates/cli/tests/publish.rs @@ -80,7 +80,7 @@ fn cli_can_publish_no_conflict_without_delete_data_flag() { fn cli_can_publish_with_automigration_change() { migration_test( "automigration-test", - &["--build-options=--features test-add-column", "--yes-break-clients"], + &["--build-options=--features test-add-column", "--break-clients"], true, ); } @@ -103,7 +103,7 @@ fn cli_can_publish_automigration_change_with_on_conflict_and_yes_break_clients() // NOTE: deleting data requires --yes, // so not providing it here ensures that no data deletion is attempted. "--delete-data=on-conflict", - "--yes-break-clients", + "--break-clients", ], true, ); @@ -140,7 +140,7 @@ fn cli_can_publish_automigration_change_with_delete_data_always_and_yes_break_cl "--build-options=--features test-add-column", "--delete-data", "--yes", - "--yes-break-clients", + "--break-clients", ], true, ); From 0016ef71cb557fea14ca858bf7a3dcbefefd92c5 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 25 Nov 2025 17:57:37 -0800 Subject: [PATCH 14/20] [tyler/fix-on-conflict]: hide --features --- crates/cli/src/subcommands/build.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/cli/src/subcommands/build.rs b/crates/cli/src/subcommands/build.rs index 411056cd405..ba63da5b59d 100644 --- a/crates/cli/src/subcommands/build.rs +++ b/crates/cli/src/subcommands/build.rs @@ -28,6 +28,7 @@ pub fn cli() -> clap::Command { .value_parser(clap::value_parser!(OsString)) .required(false) .help("Additional features to pass to the build process (e.g. `--features feature1,feature2` for Rust modules).") + .hide(true) ) .arg( Arg::new("debug") From 4724cb824596fbb46b40b45d13e8193c827bb684 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 25 Nov 2025 18:36:03 -0800 Subject: [PATCH 15/20] [tyler/fix-on-conflict]: try undoing the unsetting of CARGO_* --- crates/cli/tests/util.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/crates/cli/tests/util.rs b/crates/cli/tests/util.rs index 182fb0672f1..1bb5a99fb15 100644 --- a/crates/cli/tests/util.rs +++ b/crates/cli/tests/util.rs @@ -23,14 +23,6 @@ pub struct SpacetimeDbGuard { // Remove all Cargo-provided env vars from a child process. These are set by the fact that we're running in a cargo // command (e.g. `cargo test`). We don't want to inherit any of these to a child cargo process, because it causes // unnecessary rebuilds. -fn unset_cargo_env_vars(cmd: &mut Command) { - for (key, _) in std::env::vars() { - if key.starts_with("CARGO_") && key != "CARGO_TARGET_DIR" { - cmd.env_remove(&key); - } - } -} - impl SpacetimeDbGuard { /// Start `spacetimedb` in a temporary data directory via: /// cargo run -p spacetimedb-cli -- start --data-dir --listen-addr @@ -72,7 +64,6 @@ impl SpacetimeDbGuard { for pkg in targets { let mut cmd = Command::new("cargo"); - unset_cargo_env_vars(&mut cmd); let _ = cmd .args(["build", "-p", pkg]) .current_dir(workspace_dir) @@ -83,7 +74,6 @@ impl SpacetimeDbGuard { fn spawn_child(workspace_dir: &str, args: &[&str]) -> (Child, Arc>) { let mut cmd = Command::new("cargo"); - unset_cargo_env_vars(&mut cmd); let mut child = cmd .args(args) .current_dir(workspace_dir) From c146cfeda5807d8b90b646051d549d01a79a832a Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 25 Nov 2025 19:50:06 -0800 Subject: [PATCH 16/20] [tyler/fix-on-conflict]: try revert --- crates/cli/src/subcommands/publish.rs | 95 +++++++++++++-------------- 1 file changed, 47 insertions(+), 48 deletions(-) diff --git a/crates/cli/src/subcommands/publish.rs b/crates/cli/src/subcommands/publish.rs index ad94a3f4d84..83e4813fda0 100644 --- a/crates/cli/src/subcommands/publish.rs +++ b/crates/cli/src/subcommands/publish.rs @@ -102,6 +102,26 @@ i.e. only lowercase ASCII letters and numbers, separated by dashes."), .after_help("Run `spacetime help publish` for more detailed information.") } +fn prompt_and_clear( + name_or_identity: &str, + force: bool, + mut builder: reqwest::RequestBuilder, +) -> Result { + println!( + "This will DESTROY the current {} module, and ALL corresponding data.", + name_or_identity + ); + if !y_or_n( + force, + format!("Are you sure you want to proceed? [deleting {}]", name_or_identity).as_str(), + )? { + anyhow::bail!("Aborted."); + } + + builder = builder.query(&[("clear", true)]); + Ok(builder) +} + pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> { let server = args.get_one::("server").map(|s| s.as_str()); let name_or_identity = args.get_one::("name|identity"); @@ -175,20 +195,24 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E let domain = percent_encoding::percent_encode(name_or_identity.as_bytes(), encode_set); let mut builder = client.put(format!("{database_host}/v1/database/{domain}")); - builder = apply_pre_publish_if_needed( - builder, - &client, - &database_host, - name_or_identity, - &domain.to_string(), - host_type, - &program_bytes, - &auth_header, - clear_database, - force_break_clients, - force, - ) - .await?; + if clear_database == ClearMode::Always { + builder = prompt_and_clear(name_or_identity, force, builder)?; + } else { + builder = apply_pre_publish_if_needed( + builder, + &client, + &database_host, + &name_or_identity, + &domain.to_string(), + host_type, + &program_bytes, + &auth_header, + clear_database, + force_break_clients, + force, + ) + .await?; + } builder } else { @@ -323,6 +347,9 @@ async fn apply_pre_publish_if_needed( force_break_clients: bool, force: bool, ) -> Result { + // The caller enforces this + assert!(clear_database != ClearMode::Always); + if let Some(pre) = call_pre_publish( client, base_url, @@ -335,48 +362,20 @@ async fn apply_pre_publish_if_needed( { match pre { PrePublishResult::ManualMigrate(manual) => { - if clear_database == ClearMode::OnConflict { - println!("{}", manual.reason); - println!("Proceeding with database clear due to --delete-data=on-conflict."); - } if clear_database == ClearMode::Never { println!("{}", manual.reason); println!("Aborting publish due to required manual migration."); anyhow::bail!("Aborting because publishing would require manual migration or deletion of data and --delete-data was not specified."); } - if clear_database == ClearMode::Always { - println!("{}", manual.reason); - println!("Proceeding with database clear due to --delete-data=always."); - } - println!( - "This will DESTROY the current {} module, and ALL corresponding data.", - name_or_identity - ); - if !y_or_n( - force, - format!("Are you sure you want to proceed? [deleting {}]", name_or_identity).as_str(), - )? { - anyhow::bail!("Aborting"); - } - builder = builder.query(&[("clear", true)]); + println!("{}", manual.reason); + println!("Proceeding with database clear due to --delete-data=on-conflict."); + + builder = prompt_and_clear(name_or_identity, force, builder)?; } PrePublishResult::AutoMigrate(auto) => { - if clear_database == ClearMode::Always { - println!("Auto-migration, does NOT require clearing the database, but proceeding with database clear due to --delete-data=always."); - println!( - "This will DESTROY the current {} module, and ALL corresponding data.", - name_or_identity - ); - if !y_or_n( - force, - format!("Are you sure you want to proceed? [deleting {}]", name_or_identity).as_str(), - )? { - anyhow::bail!("Aborting"); - } - builder = builder.query(&[("clear", true)]); - return Ok(builder); - } println!("{}", auto.migrate_plan); + // We only arrive here if you have not specified ClearMode::Always AND there was no + // conflict that required manual migration. if auto.break_clients && !y_or_n( force_break_clients || force, From 76f0995d3a5f188ab416e40f2e45d86238f4a192 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Tue, 25 Nov 2025 19:51:03 -0800 Subject: [PATCH 17/20] [tyler/fix-on-conflict]: rename --- crates/cli/src/subcommands/publish.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/cli/src/subcommands/publish.rs b/crates/cli/src/subcommands/publish.rs index 83e4813fda0..2f590e40642 100644 --- a/crates/cli/src/subcommands/publish.rs +++ b/crates/cli/src/subcommands/publish.rs @@ -102,7 +102,7 @@ i.e. only lowercase ASCII letters and numbers, separated by dashes."), .after_help("Run `spacetime help publish` for more detailed information.") } -fn prompt_and_clear( +fn confirm_and_clear( name_or_identity: &str, force: bool, mut builder: reqwest::RequestBuilder, @@ -196,7 +196,7 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E let mut builder = client.put(format!("{database_host}/v1/database/{domain}")); if clear_database == ClearMode::Always { - builder = prompt_and_clear(name_or_identity, force, builder)?; + builder = confirm_and_clear(name_or_identity, force, builder)?; } else { builder = apply_pre_publish_if_needed( builder, @@ -370,7 +370,7 @@ async fn apply_pre_publish_if_needed( println!("{}", manual.reason); println!("Proceeding with database clear due to --delete-data=on-conflict."); - builder = prompt_and_clear(name_or_identity, force, builder)?; + builder = confirm_and_clear(name_or_identity, force, builder)?; } PrePublishResult::AutoMigrate(auto) => { println!("{}", auto.migrate_plan); From 6e4e25b8bbcc9c88276bd4d267df1fee081a41c8 Mon Sep 17 00:00:00 2001 From: John Detter <4099508+jdetter@users.noreply.github.com> Date: Tue, 25 Nov 2025 22:31:23 -0600 Subject: [PATCH 18/20] Fix clippy lint --- crates/cli/src/subcommands/publish.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cli/src/subcommands/publish.rs b/crates/cli/src/subcommands/publish.rs index 2f590e40642..aaf16d6159a 100644 --- a/crates/cli/src/subcommands/publish.rs +++ b/crates/cli/src/subcommands/publish.rs @@ -202,7 +202,7 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E builder, &client, &database_host, - &name_or_identity, + name_or_identity, &domain.to_string(), host_type, &program_bytes, From e8f0dae216fa08c65dadec357c66703579f7556e Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Wed, 26 Nov 2025 10:03:32 -0800 Subject: [PATCH 19/20] [tyler/fix-on-conflict]: TODO --- crates/cli/src/subcommands/build.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/cli/src/subcommands/build.rs b/crates/cli/src/subcommands/build.rs index ba63da5b59d..79ef9082ed9 100644 --- a/crates/cli/src/subcommands/build.rs +++ b/crates/cli/src/subcommands/build.rs @@ -23,11 +23,13 @@ pub fn cli() -> clap::Command { .help("The directory to lint for nonfunctional print statements. If set to the empty string, skips linting.") ) .arg( + // TODO: Make this into --extra-build-args (or something similar) that will get passed along to the language's compiler. Arg::new("features") .long("features") .value_parser(clap::value_parser!(OsString)) .required(false) .help("Additional features to pass to the build process (e.g. `--features feature1,feature2` for Rust modules).") + // We're hiding this because we think it deserves a refactor first (see the TODO above) .hide(true) ) .arg( From 7d5bc316b88c7101c9777384897fbcf91275da56 Mon Sep 17 00:00:00 2001 From: Zeke Foppa Date: Wed, 26 Nov 2025 10:12:13 -0800 Subject: [PATCH 20/20] [bfops/repro-503]: Revert --- crates/cli/src/subcommands/publish.rs | 95 ++++++++++++++------------- 1 file changed, 48 insertions(+), 47 deletions(-) diff --git a/crates/cli/src/subcommands/publish.rs b/crates/cli/src/subcommands/publish.rs index aaf16d6159a..ad94a3f4d84 100644 --- a/crates/cli/src/subcommands/publish.rs +++ b/crates/cli/src/subcommands/publish.rs @@ -102,26 +102,6 @@ i.e. only lowercase ASCII letters and numbers, separated by dashes."), .after_help("Run `spacetime help publish` for more detailed information.") } -fn confirm_and_clear( - name_or_identity: &str, - force: bool, - mut builder: reqwest::RequestBuilder, -) -> Result { - println!( - "This will DESTROY the current {} module, and ALL corresponding data.", - name_or_identity - ); - if !y_or_n( - force, - format!("Are you sure you want to proceed? [deleting {}]", name_or_identity).as_str(), - )? { - anyhow::bail!("Aborted."); - } - - builder = builder.query(&[("clear", true)]); - Ok(builder) -} - pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::Error> { let server = args.get_one::("server").map(|s| s.as_str()); let name_or_identity = args.get_one::("name|identity"); @@ -195,24 +175,20 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E let domain = percent_encoding::percent_encode(name_or_identity.as_bytes(), encode_set); let mut builder = client.put(format!("{database_host}/v1/database/{domain}")); - if clear_database == ClearMode::Always { - builder = confirm_and_clear(name_or_identity, force, builder)?; - } else { - builder = apply_pre_publish_if_needed( - builder, - &client, - &database_host, - name_or_identity, - &domain.to_string(), - host_type, - &program_bytes, - &auth_header, - clear_database, - force_break_clients, - force, - ) - .await?; - } + builder = apply_pre_publish_if_needed( + builder, + &client, + &database_host, + name_or_identity, + &domain.to_string(), + host_type, + &program_bytes, + &auth_header, + clear_database, + force_break_clients, + force, + ) + .await?; builder } else { @@ -347,9 +323,6 @@ async fn apply_pre_publish_if_needed( force_break_clients: bool, force: bool, ) -> Result { - // The caller enforces this - assert!(clear_database != ClearMode::Always); - if let Some(pre) = call_pre_publish( client, base_url, @@ -362,20 +335,48 @@ async fn apply_pre_publish_if_needed( { match pre { PrePublishResult::ManualMigrate(manual) => { + if clear_database == ClearMode::OnConflict { + println!("{}", manual.reason); + println!("Proceeding with database clear due to --delete-data=on-conflict."); + } if clear_database == ClearMode::Never { println!("{}", manual.reason); println!("Aborting publish due to required manual migration."); anyhow::bail!("Aborting because publishing would require manual migration or deletion of data and --delete-data was not specified."); } - println!("{}", manual.reason); - println!("Proceeding with database clear due to --delete-data=on-conflict."); - - builder = confirm_and_clear(name_or_identity, force, builder)?; + if clear_database == ClearMode::Always { + println!("{}", manual.reason); + println!("Proceeding with database clear due to --delete-data=always."); + } + println!( + "This will DESTROY the current {} module, and ALL corresponding data.", + name_or_identity + ); + if !y_or_n( + force, + format!("Are you sure you want to proceed? [deleting {}]", name_or_identity).as_str(), + )? { + anyhow::bail!("Aborting"); + } + builder = builder.query(&[("clear", true)]); } PrePublishResult::AutoMigrate(auto) => { + if clear_database == ClearMode::Always { + println!("Auto-migration, does NOT require clearing the database, but proceeding with database clear due to --delete-data=always."); + println!( + "This will DESTROY the current {} module, and ALL corresponding data.", + name_or_identity + ); + if !y_or_n( + force, + format!("Are you sure you want to proceed? [deleting {}]", name_or_identity).as_str(), + )? { + anyhow::bail!("Aborting"); + } + builder = builder.query(&[("clear", true)]); + return Ok(builder); + } println!("{}", auto.migrate_plan); - // We only arrive here if you have not specified ClearMode::Always AND there was no - // conflict that required manual migration. if auto.break_clients && !y_or_n( force_break_clients || force,