Skip to content

Smoketests for default values - #5618

Open
bfops wants to merge 33 commits into
masterfrom
bfops/consolidate-default-tests
Open

Smoketests for default values#5618
bfops wants to merge 33 commits into
masterfrom
bfops/consolidate-default-tests

Conversation

@bfops

@bfops bfops commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Description of Changes

Motivated by #5562, this PR adds smoketests for default column values in all suported languages.

This uncovered multiple bugs:

Follow-up PR #5630 fixes three of those bugs.

API and ABI breaking changes

None. New tests only.

Expected complexity level and risk

2

Testing

  • CI passes
  • CI correctly failed on broken default values

@bfops
bfops changed the base branch from master to bfops/test-suite-fixes July 29, 2026 17:58
@bfops bfops changed the title [Spelunking] C++/Rust schema compare tests Smoketests for default values Jul 29, 2026
@bfops
bfops marked this pull request as ready for review July 29, 2026 19:18
Comment thread crates/smoketests/tests/smoketests/column_defaults.rs Outdated
@bfops
bfops changed the base branch from bfops/test-suite-fixes to master July 30, 2026 03:59
@bfops
bfops requested a review from gefjon July 30, 2026 14:27
Comment on lines +8 to +44
// TODO: uncomment this once negative defaults are fixed in Rust
// https://github.com/clockworklabs/SpacetimeDB/issues/5622
//("i8_value", "-8"),
("u16_value", "16"),
// TODO: uncomment this once negative defaults are fixed in Rust
// https://github.com/clockworklabs/SpacetimeDB/issues/5622
//("i16_value", "-16"),
("u32_value", "32"),
// TODO: uncomment this once negative defaults are fixed in Rust
// https://github.com/clockworklabs/SpacetimeDB/issues/5622
//("i32_value", "-32"),

// TODO: uncomment once u64s are fixed.
// https://github.com/clockworklabs/SpacetimeDB/issues/5623
//("u64_value", "64"),

// TODO: uncomment this once negative defaults are fixed in Rust
// https://github.com/clockworklabs/SpacetimeDB/issues/5622
//("i64_value", "-64"),

// TODO: uncomment once floats are fixed in C# and f32s are fixed in Rust
// https://github.com/clockworklabs/SpacetimeDB/issues/5624
// https://github.com/clockworklabs/SpacetimeDB/issues/5627
//("f32_positive_value", "32.5"),

// TODO: uncomment once floats are fixed in C#, f32s are fixed in Rust, and negative defaults are fixed in Rust
// https://github.com/clockworklabs/SpacetimeDB/issues/5624
// https://github.com/clockworklabs/SpacetimeDB/issues/5627
// https://github.com/clockworklabs/SpacetimeDB/issues/5622
//("f32_negative_value", "-32.5"),
("f64_positive_value", "64.25"),
// TODO: uncomment once negative defaults are fixed in Rust
// https://github.com/clockworklabs/SpacetimeDB/issues/5622
//("f64_negative_value", "-64.25"),

// TODO: uncomment this once string default values are fixed in Rust
//("string_value", r#""default string""#),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For any of these commented-out cases that you don't intend to fix before merging this PR, please update the issue to mention that the test should be uncommented when completed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fixes in #5630 turned out to be pretty small, and you discovered that we've actually fixed all 4 bugs - what do you think about merging #5630 into this one?

random_string, require_dotnet, require_emscripten, require_pnpm, ModuleLanguage, Smoketest,
};

const EXPECTED_DEFAULTS: &[(&str, &str)] = &[

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If support varies by language, can we have multiple different EXPECTED_DEFAULTS for the different languages? It seems silly to not test string defaults in C# or TypeScript just because we don't currently support them in Rust.

@bfops bfops Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, although I'd quibble about implementation. I would prefer to configure each case to have a list of skipped languages, so it is harder to accidentally quietly elide a case from a language's list entirely.

But given that we have small open PRs that address all the issues, I'm kind of inclined to leave this as-is since they will unify pretty imminently. Does that seem reasonable to you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants