简单点：为它写个 Test
简单点：为它写个 Test
2025年3月25日 | Reddit 讨论
这是一篇简短的赞赏文章，关于 Rust 如何不断引导我朝着做正确的事情™的方向前进。
Google Summer of Code 2025 即将到来，这意味着一大批新的贡献者开始向各种 Rust 项目发送 pull requests。其中最受欢迎的项目之一似乎是 bors，这是一个从头开始实现的 merge queue bot，我们的目标是用它来管理主要的 Rust compiler repository 中的 pull requests。
在过去的几周里，我审查并合并了 bors 中的数十个 PR。 可惜的是，我很快就发现其中一些破坏了该 bot 的（staging）部署。 这非常烦人，因为我一直在刻意地设计该 bot、tests 和 CI，以避免发生类似的事情。 在实践中，我们可能会在 staging 环境中检测到此问题，因此希望它不会影响生产环境，但这仍然是我希望尽可能避免的情况。
经过一番调查后，我意识到问题出在一个 SQL migration 中，它看起来像这样：
ALTER TABLE foo ADD COLUMN bar NOT NULL;
乍一看似乎很正常，直到我意识到向已填充的表中添加一个 NOT NULL 列，而不为现有行提供一些 DEFAULT 值，并不是一个好主意。 现有的 test suite 没有发现这个问题（即使它运行了近 200 个端到端 tests），因为它总是从一个空数据库开始，应用所有 migrations，然后才运行 test 代码。
我修复了这个 bug，但并不想就此止步。我的编程经验1教会我（几乎）总是尝试弄清楚如何确保在首次修复某个 bug（或理想情况下是一整类 bug）后，它不会再次发生。 在这种情况下，起初似乎有点棘手，因为问题出在任意 SQL 语句的结构中。
我首先在 bors 开发指南中添加了一个 warning，敦促人们不要编写这样的 migrations。 虽然这比没有好，但很明显，在实践中，仅靠文档对于防止类似 bug 的作用非常有限。
如果这是任何其他技术或语言，我很可能会就此打住，然后就此作罢。 但是使用 Rust，我不知何故感到鼓励（并被赋予了力量！）去加倍努力，并尝试确保我已尽一切努力来防止未来的问题（以及在出现问题时发出的紧急 ping :) ）。
但是我们在这里能做什么呢？ 当然，我们不会_解析和检查 SQL 查询_仅仅为了一个 test 吧？
...嗯，经过一段时间的思考，为什么我们不能呢？ 我知道有一个用于解析 SQL 的 crate 叫做 sqlparser。 我担心它会有数百个依赖项，并且对于编写单个 test 来说太过分了，但是当我将其添加为 dev 依赖项时，我发现它只有大约三个微小的依赖项（甚至可以在需要时禁用），并且编译速度很快。
站在巨人的肩膀上，并配备了生产级的 SQL parser，我开始编写一个 integration test，它遍历 migrations 目录，解析每个 SQL 文件，并检测在没有 DEFAULT 子句的情况下添加 NOT NULL 列的情况。 sqlparser 支持 Visitor 模式，这使得实现变得非常容易。 我的解决方案可能并非万无一失，并且肯定会遗漏一些情况，但它应该足以在典型的 migration 查询中找到有问题的状况。
这篇博文的目的不是展示如何使用 sqlparser，因此我不会深入探讨它，但是如果您有兴趣，可以查看下面的完整 test 代码（仅大约 100 行代码，不包括 imports）：
Test code
use itertools::Itertools;
use sqlparser::ast::{
AlterColumnOperation, AlterTableOperation, ColumnOption, Ident, ObjectName,
Statement, Visit, Visitor,
};
use sqlparser::dialect::PostgreSqlDialect;
use sqlparser::parser::Parser;
use std::collections::{BTreeSet, HashSet};
use std::ffi::OsStr;
use std::ops::ControlFlow;
use std::path::PathBuf;
struct CheckNotNullWithoutDefault {
error: Option<String>,
columns_set_to_not_null: HashSet<(ObjectName, Ident)>,
columns_set_default_value: HashSet<(ObjectName, Ident)>,
}
impl Visitor for CheckNotNullWithoutDefault {
type Break = ();
fn pre_visit_statement(&mut self, statement: &Statement) -> ControlFlow<Self::Break> {
if let Statement::AlterTable {
operations, name, ..
} = statement
{
for op in operations {
match op {
AlterTableOperation::AddColumn { column_def, .. } => {
let has_not_null = column_def
.options
.iter()
.any(|option| option.option == ColumnOption::NotNull);
let has_default = column_def
.options
.iter()
.any(|option| matches!(option.option, ColumnOption::Default(_)));
if has_not_null && !has_default {
self.error = Some(format!(
"Column `{name}.{}` is NOT NULL, but no DEFAULT value was configured!",
column_def.name
));
return ControlFlow::Break(());
}
}
AlterTableOperation::AlterColumn { column_name, op } => match op {
AlterColumnOperation::SetNotNull => {
self.columns_set_to_not_null
.insert((name.clone(), column_name.clone()));
}
AlterColumnOperation::SetDefault { .. } => {
self.columns_set_default_value
.insert((name.clone(), column_name.clone()));
}
_ => {}
},
_ => {}
}
}
}
ControlFlow::Continue(())
}
}
impl CheckNotNullWithoutDefault {
fn compute_error(self) -> Option<String> {
if let Some(error) = self.error {
return Some(error);
}
let missing_default = self
.columns_set_to_not_null
.difference(&self.columns_set_default_value)
.collect::<BTreeSet<_>>();
if !missing_default.is_empty() {
return Some(format!(
"Column(s) {} were modified to NOT NULL, but no DEFAULT value was set for them",
missing_default.iter().map(|v| format!("{v:?}")).join(",")
));
}
None
}
}
/// Check that there is no migration that would add a NOT NULL column (or make an existing column
/// NOT NULL) without also providing a DEFAULT value.
#[test]
fn check_non_null_column_without_default() {
let root = env!("CARGO_MANIFEST_DIR");
let migrations = PathBuf::from(root).join("migrations");
for file in std::fs::read_dir(migrations).unwrap() {
let file = file.unwrap();
if file.path().extension() == Some(OsStr::new("sql")) {
let contents =
std::fs::read_to_string(&file.path()).expect("cannot read migration file");
let ast = Parser::parse_sql(&PostgreSqlDialect {}, &contents).expect(&format!(
"Cannot parse migration {} as SQLL",
file.path().display()
));
let mut visitor = CheckNotNullWithoutDefault {
error: None,
columns_set_to_not_null: Default::default(),
columns_set_default_value: Default::default(),
};
ast.visit(&mut visitor);
if let Some(error) = visitor.compute_error() {
panic!(
"Migration {} contains error: {error}",
file.path().display()
);
}
}
}
}
正如 Rust 的典型情况一样，该 test 首次尝试就起作用了。 但更令我惊讶的是我能够如此简单地实现所有这些。 从想到“我_真的可以解析_ SQL 吗？”到获得一个有效的 test，我只花了大约 10 分钟，而且使用的是一个我以前从未用过的 crate。 我什至没有阅读文档，只是复制了一行代码来引导解析过程。 我只是通过检查和遵循 autocompletion 提示2来构建了这个 test。 当你可以使用出色的 type system 和强大的 IDE 进行 vibe coding 时，谁还需要 AI 呢 :)
如果您好奇，该修复和 test 已在 这个 PR 中实现。 除了解析 SQL 查询之外，我还考虑了另一种测试方法，我可能会在将来实现：逐个遍历每个 migration，并在应用它之前将一些 dummy data 插入到数据库中，以确保我们测试每个 migration 都在非空数据库上应用。 这些数据要么必须根据当前数据库 schema 自动生成，要么我们可以将一些示例 DB dataset 与每个 migration 一起提交，以确保我们有一些具有代表性的数据样本可用。
因此，下次当您想知道“我应该编写一个 test 还是希望这种情况永远不会再发生？”时，请尝试编写该 test，即使起初听起来很烦人。 使用 Rust（和一些 crates），它可能并没有那么困难 :)
如果您有任何意见，请在 Reddit 上告诉我。
- 以及 matklad 的 blog posts。 ↩
- old-school auto-completion，不涉及任何 AI :) 嗯，IntelliJ 确实使用一些 machine learning 来（重新）排序 autocompletion 结果，但这与使用实际的 LLM 相去甚远。 ↩