frappe-code-review

Compare original and translation side by side

🇺🇸

Original

English

🇨🇳

Translation

Chinese

Quality Code Review

高质量代码审查

A reviewer's checklist for Frappe applications. Protect correctness, security, and the future maintainer, in that order of consequence. The sections below run in that order — spend most attention on §1 and §2. Prefer a root-cause fix over a workaround, and say why a finding matters (what breaks, for whom).

审查 Frappe 应用的检查清单。按后果严重程度，依次保障正确性、安全性以及未来的维护者。以下各节也按此顺序排列——应将大部分注意力放在第 1 节和第 2 节。优先采用根因修复，而不是变通方案，并说明某项发现为何重要（会造成什么后果、影响哪些人）。

1. Correctness & stability (highest consequence)

1. 正确性与稳定性（后果最严重）

The worst bug is silent stateful corruption — wrong ledgers/stock posted with no error. Treat stateful and legal/accounting/compliance code as "failure is not an option" code.

Fail early and loudly. Use assertions for internal invariants:
```
assert total_credit == total_debit
```
. (Assertions are for invariants the code guarantees — not user-facing validation.)
Picture how it breaks. For every change ask: Where can this break? How will someone misuse this? Write fool-proof code. Extensions and overrides should especially consider handling all sorts of failure modes.
No partial commits. A stray
```
frappe.db.commit()
```
/
```
db.rollback()
```
mid-transaction ends the transaction and exposes partial state — flag every one. Submitting/saving with validation bypassed (docs posted with no SL/GL entry when validation fails) is a critical bug.
Don't sacrifice atomicity for convenience (e.g. adding
```
autocommit
```
to fix bootstrapping makes transactions non-atomic). Autocommit belongs only on schema creation.
Preserve invariants over UX. "Compromise UX, but guarantee correctness." Don't degrade working code to accommodate broken code.
Validate the issue before fixing it. Sometimes the correct fix is "don't fix this". Identify the root causes first.
Watch for destructive DB APIs with empty/
None
filters.
```
set_value("Site", None, ...)
```
/
```
db.delete
```
with no filter updates/deletes every row. These must error, not silently operate on the whole table. Flag any
```
set_value
```
/
```
delete
```
/
```
get_value
```
where the name/filter could be
```
None
```
or empty- or attacker-controlled.
Check the types in a condition actually match. A comparison between mismatched types (string vs
```
datetime
```
, string vs int) silently never matches or is always true — cast explicitly (
```
cint
```
/
```
flt
```
) at the boundary.
Question the return shape. Before indexing a result, ask whether it can be
```
None
```
/
```
[None, None]
```
; watch
```
as_dict=1
```
(list of dicts) vs scalar confusion.
Don't silently change long-standing semantics. Behavior callers have relied on for a long time is a contract — altering it is a breaking change in disguise, even when no signature changed.

最糟糕的 bug 是静默的有状态数据损坏——错误的账本/库存被过账，却没有报错。将有状态代码以及法律/会计/合规代码视为**“绝不能失败”**代码。

尽早且显式地失败。 对内部不变量使用断言：
```
assert total_credit == total_debit
```
。（断言用于代码自身保证的不变量，而不是面向用户的验证。）
设想过会如何出错。 对每项改动都要问：它会在哪里出错？他人会如何误用？ 编写难以误用的代码。扩展和覆盖尤其应考虑处理各种失败模式。
不允许部分提交。 事务中途意外调用
```
frappe.db.commit()
```
/
```
db.rollback()
```
会结束事务并暴露部分状态——每一处都应标记。在绕过验证的情况下提交/保存（验证失败时单据仍被过账且没有 SL/GL 分录）是严重 bug。
不要为了便利牺牲原子性（例如，为解决引导启动问题而添加
```
autocommit
```
会使事务失去原子性）。
```
autocommit
```
只应用于创建 schema。
优先保障不变量，而非用户体验。 “可以牺牲 UX，但必须保证正确性。” 不要为了迁就有问题的代码而降低原本正常工作的代码质量。
修复前先验证问题。 有时正确的修复是“不要修复”。先找出根因。
警惕过滤条件为空或为
None
的破坏性数据库 API。
```
set_value("Site", None, ...)
```
/ 不带过滤条件的
```
db.delete
```
会更新/删除每一行。这些操作必须报错，而不是悄无声息地作用于整张表。标记所有名称/过滤条件可能为
```
None
```
、为空或可被攻击者控制的
```
set_value
```
/
```
delete
```
/
```
get_value
```
。
检查条件中的类型是否真正匹配。 类型不匹配的比较（字符串与
```
datetime
```
、字符串与整数）会静默地永远不匹配或始终成立——应在边界处显式转换（
```
cint
```
/
```
flt
```
）。
质疑返回结构。 在对结果进行索引前，先判断它是否可能为
```
None
```
/
```
[None, None]
```
；注意
```
as_dict=1
```
（字典列表）与标量之间的混淆。
不要静默改变长期以来的语义。 调用方长期依赖的行为就是契约——即使签名未变，改变它也等同于隐藏了破坏性变更。

2. Security

2. 安全性

Avoiding a vulnerability is far easier than fixing one safely. Audit security-critical code (auth, authorization, permissions, user management) especially hard.

Injection

NEVER build SQL by string concatenation/f-strings. Use the ORM or query builder. If raw SQL is unavoidable, use parameter substitution (
```
frappe.db.sql("... where name = %s", (user,))
```
) — never interpolate yourself. Better still, avoid introducing new raw SQL at all: beyond injection risk it ties code to one database, and the framework aims to stay DB-agnostic (Postgres support). Prefer
```
frappe.qb
```
.
Type confusion is an injection vector even with the ORM. Frappe accepts complex types, so a parameter expected to be a string can arrive as a filter list:
```
{"key": ["!=", ""]}
```
passed to
```
db.get_value
```
bypasses a secret-key check. Validate input types at trust boundaries — explicit
```
isinstance(key, str)
```
. Audit every
```
@frappe.whitelist
```
method for this.
Never
```
eval
```
/
```
exec
```
anything yourself.
```
safe_eval
```
/
```
safe_exec
```
only, in limited volume, and "safe_exec is not magic." Never accept a client-supplied method path to execute.

Sandboxing & trust boundaries

Sandboxed execution (RestrictedPython/
```
safe_exec
```
) is not reliably safe — assume escapes exist. Security toggles must live at the right trust boundary: server-script enablement is a bench-level config, never site-level (a tenant could enable it and take over the whole server).
Prefer allowlists over blocklists — blocklists are bypassable. Don't expose everything by default.

Access control

"Think 10 times before
```
allow_guest=True
```
" — it is not a shortcut around real authn/authz. Web pages must apply permissions before reading/sharing data. Prefer
```
get_list
```
/
```
get_all
```
over hand-rolled queries.
Scope relaxations precisely. Verify a rate-limit/permission exception targets exactly the intended principal — not, say, all non-guest users.

Path traversal / filesystem

Prefer the File doctype API. If user input enters a path, ensure it can't traverse (
```
/../../
```
) outside the site folder.

Crypto / secrets

Never roll your own crypto; reuse existing implementations. Verify authenticity of guest/webhook requests (HMAC).
Signed/one-time URLs: use a truly secret signing value; expire by both time and first use; validate using the URL alone, not merged form data (Frappe merges URL + form data → replay attacks with one valid signature).
Store secrets in password fields; never plain text; never leak secrets in logs or error messages.

XSS & the rest of OWASP

Don't inject user input into the DOM. Treat XSS as critical even when it looks trivial — HTML/JS injection usually leads to account hijack.
Don't fix XSS by sanitizing and throwing away special characters. Prefer escaping right before injecting values in DOM.

避免引入漏洞远比事后安全修复容易。应特别严格审计安全关键代码（认证、授权、权限、用户管理）。

注入

绝不通过字符串拼接/f-string 构造 SQL。 使用 ORM 或 query builder。若不可避免地需要原始 SQL，则使用参数替换（
```
frappe.db.sql("... where name = %s", (user,))
```
）——绝不自行插值。更好的是，完全避免引入新的原始 SQL：除了注入风险外，它还会将代码绑定到单一数据库，而框架的目标是保持数据库无关（支持 Postgres）。优先使用
```
frappe.qb
```
。
即使使用 ORM，类型混淆也是注入向量。 Frappe 接受复杂类型，因此预期为字符串的参数可能以过滤器列表的形式到达：传给
```
db.get_value
```
的
```
{"key": ["!=", ""]}
```
可绕过密钥检查。应在信任边界验证输入类型——显式使用
```
isinstance(key, str)
```
。审计每个
```
@frappe.whitelist
```
方法。
绝不自行
```
eval
```
/
```
exec
```
任何内容。仅可有限度地使用
```
safe_eval
```
/
```
safe_exec
```
，而且“safe_exec 并不是魔法”。绝不接受由客户端提供的方法执行路径。

沙箱与信任边界

沙箱执行（RestrictedPython/
```
safe_exec
```
）无法被可靠地视为安全——应假定存在逃逸手段。安全开关必须位于正确的信任边界：服务器脚本启用权限应是 bench 级配置，绝不能是站点级配置（租户可能借此控制整台服务器）。
优先使用 allowlist 而不是 blocklist——blocklist 可被绕过。不要默认公开所有内容。

访问控制

“在设置
```
allow_guest=True
```
前要思考 10 次”——它不是绕过真正 authn/authz 的捷径。网页必须先应用权限，再读取/分享数据。优先使用
```
get_list
```
/
```
get_all
```
，而不是手写查询。
精确限定权限放宽范围。 验证限流/权限例外所针对的主体恰好是预期主体——例如不能面向所有非访客用户。

路径遍历/文件系统

优先使用 File doctype API。如果用户输入进入路径，请确保其无法通过路径遍历（
```
/../../
```
）逃逸到站点目录之外。

加密/机密

绝不自行实现加密；复用现有实现。验证访客/Webhook 请求的真实性（HMAC）。
签名/一次性 URL：使用真正保密的签名值；同时按时间和首次使用限制过期；仅使用 URL 验证，不要与合并后的表单数据一起验证（Frappe 会合并 URL + 表单数据 → 一个有效签名可被重放攻击）。
将机密存储在 password 字段中；绝不明文存储；绝不在日志或错误消息中泄露机密。

XSS 与 OWASP 的其他部分

不要将用户输入注入 DOM。即使看似轻微，也应将 XSS 视为严重问题——HTML/JS 注入通常会导致账户劫持。
不要通过清洗并丢弃特殊字符来修复 XSS。优先在将值注入 DOM 前进行转义。

3. Performance is correctness

3. 性能就是正确性

The cheapest time to fix performance is at review; slow code merged sits undetected for years. Performance is a feature (Doherty threshold; humans perceive ~100ms).

Budgets: common reads < 100ms; reads < 1s; most writes < 5s; never exceed ~10s. P99 of a frequent read-only request should be ~1s. A slow synchronous request blocks a worker (head-of-line blocking).
Complexity rule: a frequently-called endpoint must do O(1) or O(log N) work — a large constant factor at worst, never O(N). Counting rows is O(N), not O(1);
```
COUNT(*)
```
over a large/filtered table is expensive. Bound unbounded scans (e.g. last 3 months, a "1000+" sentinel) rather than scanning everything.
Indexes are code. Flag any
```
WHERE
```
/join/filter on an unindexed column. Indexes (and custom indexes) must be committed in code, not applied ad-hoc — they get lost on migration otherwise. Form loads that pull comments/versions/assignments need all those queries indexed; one unindexed query makes everything sluggish.
No DB calls in loops. "Don't write validations that call db in LOOPS." Flag N+1 patterns. Cache stable values (UOM, docstatus, status) instead of re-querying. This is acceptable in background jobs, but never in requests.
The Remove → Reduce → Reuse ladder for slow code you can't fix: remove it, invoke it less, or memoize. Pick the right cache scope (
```
@redis_cache
```
,
```
@request_cache
```
,
```
@site_cache
```
— the last balloons memory if overused). DO NOT hand-roll caches in
```
frappe.local
```
or
```
frappe.flags
```
: "you'll just be creating brand-new cache-invalidation bugs." Don't cache trivially cheap work.
Memory: don't stuff junk into shared module-level files /
```
__init__.py
```
/ class-level state — it stays resident forever. Remove unused module-level imports (move into the function that uses them). Watch for leaks.
Reorder conditionals so the DB call is last. In a boolean expression, put cheap in-memory checks first so short-circuiting can skip the query entirely.
Aggregate in SQL, not Python. Use
```
SUM()
```
/
```
COUNT()
```
in the query instead of fetching all rows to reduce them in memory; push filters into the subquery so they apply before the join.
Don't fetch a whole doc for one value. Use
```
get_value
```
/
```
get_single_value
```
/
```
set_value
```
for a single column instead of
```
get_doc().save()
```
; use
```
frappe.delete_doc
```
instead of
```
get_doc().delete()
```
(which fetches the doc only to delete it).
No MyISAM tables in hot paths. Reading a MyISAM table takes an implicit table-level lock — never touch one in a request path.
Move long work to a background queue. Long-running work belongs in
```
enqueue(..., queue="long")
```
, not a synchronous request that blocks a worker.

修复性能问题的最佳时机是在审查阶段；合并进来的慢代码往往多年无人察觉。性能是一项功能（Doherty 阈值；人类感知的延迟约为 100ms）。

预算： 常见读取 < 100ms；读取 < 1s；大多数写入 < 5s；绝不超过约 10s。高频只读请求的 P99 应约为 1s。缓慢的同步请求会阻塞 worker（队头阻塞）。
复杂度规则： 高频调用的 endpoint 必须执行 O(1) 或 O(log N) 的工作量——最坏只能有较大的常数因子，绝不能是 O(N)。统计行数是 O(N)，不是 O(1)；对大型/经过筛选的表执行
```
COUNT(*)
```
代价高昂。应限制无界扫描（例如最近 3 个月，或设置“1000+”哨兵值），而不是扫描全部数据。
索引也是代码。 标记所有在未索引列上的
```
WHERE
```
/join/filter。索引（包括自定义索引）必须通过代码提交，不能临时应用——否则迁移后会丢失。加载表单时，如果会拉取评论/版本/分配信息，则这些查询必须全部建立索引；一个未索引查询就会让整体变慢。
循环中不得调用数据库。 “不要编写会在 LOOP 中调用 db 的验证。”标记 N+1 模式。缓存稳定值（UOM、docstatus、status），而不是重复查询。在后台作业中可以接受这种做法，但请求中绝不可以。
对于无法修复的慢代码，遵循移除 → 减少 → 复用阶梯：移除它、减少调用，或对它进行记忆化处理。选择正确的缓存作用域（
```
@redis_cache
```
、
```
@request_cache
```
、
```
@site_cache
```
——后者过度使用会显著膨胀内存）。不要在
```
frappe.local
```
或
```
frappe.flags
```
中自行实现缓存：“你只会制造全新的缓存失效 bug。”不要缓存成本极低的工作。
内存： 不要把无用数据塞进共享的模块级文件/
```
__init__.py
```
/类级状态——它们会永久驻留。移除未使用的模块级导入（移入使用它们的函数中）。注意内存泄漏。
调整条件顺序，让数据库调用放在最后。 在布尔表达式中，先执行开销低的内存检查，这样短路求值就能完全跳过查询。
在 SQL 中聚合，而不是在 Python 中。 在查询中使用
```
SUM()
```
/
```
COUNT()
```
，而不是取回所有行后在内存中计算；将过滤条件推入子查询，使其在 join 之前生效。
不要为了一个值加载整份文档。 单列值应使用
```
get_value
```
/
```
get_single_value
```
/
```
set_value
```
，而不是
```
get_doc().save()
```
；使用
```
frappe.delete_doc
```
，而不是
```
get_doc().delete()
```
（后者会先加载整个文档再删除）。
热路径中不要使用 MyISAM 表。 读取 MyISAM 表会获取隐式表级锁——绝不要在请求路径中访问此类表。
将耗时工作移至后台队列。 长时间运行的任务应放入
```
enqueue(..., queue="long")
```
，而不是放在会阻塞 worker 的同步请求中。

4. Concurrency

4. 并发

Check-then-act is a race.
```
if not frappe.db.exists(...): insert()
```
— two workers both see "not exists" and both insert. Prefer a DB-level unique constraint; "outsource integrity to the database."
Locking footguns:
```
SELECT ... FOR UPDATE
```
on an unindexed query locks every scanned row (and gaps) — always ensure the filter uses an index, or you lock the whole table. Locking a parent but not its children yields a "mutant" doc.
Global mutable state / class attributes are global in Python — a shared engine/class attribute leaking query state across concurrent requests produces garbage. Make query-building stateless. Don't do "weird shit with
```
frappe.local
```
" —
```
local
```
is for variables, not static state.

先检查再执行存在竞态。
```
if not frappe.db.exists(...): insert()
```
——两个 worker 都会看到“不存在”并各自插入。优先使用数据库级唯一约束；“把完整性委托给数据库。”
锁使用的陷阱： 对未建立索引的查询执行
```
SELECT ... FOR UPDATE
```
会锁住所有被扫描的行（以及间隙）——务必确保过滤条件使用索引，否则会锁住整张表。只锁定父文档却不锁子文档会产生“变异体”文档。
全局可变状态/类属性在 Python 中就是全局的——共享的 engine/class 属性在并发请求间泄漏查询状态会产生垃圾数据。让查询构建保持无状态。不要用
```
frappe.local
```
做“奇怪的事情”——
```
local
```
用于变量，而不是静态状态。

5. Readability & maintainability

5. 可读性与可维护性

~50% of dev time is spent reading code; rotten code eventually forces a rewrite.

Keep functions pure when they can be pure — easy to read and test.
Don't pass mutable objects around to be filled in ("assembly" code) — return new values. Passing a mutable to be mutated forces a reader to open two files to understand one thing.
A function that mutates its input must be named appropriately.
Prefer the boring construct. While that functional map-reduce one-liner looks beautiful, please just write a 4-line for-loop. Favor debuggable code over clever code.
Consistency over personal style. A codebase shouldn't be a hodge-podge of 10 styles. Match the surrounding formatting/naming/import conventions; flag a change that breaks them.
Good taste: restructure so the edge case becomes the common case, removing special-case branches. Ask: can this be simpler? Less code? Is it over-indented if-else soup?
Prefer extending shared components over copy-paste divergence. 3–4 forked implementations of one thing → slow long-term velocity. Avoid tight coupling across modules; integrate through clear, documented public APIs.
Document public modules/classes/functions with docstrings; prefer type annotations over describing types in prose ("type hints are 10x better"); type checkers find non-obvious bugs.
Docstrings should only mention important things. Keep them short and to the point. Don't explain what's trivially understood from function name. Focus on "why".
Split unrelated changes into separate commits/PRs — keeps review focused and
```
git blame
```
/reverts clean.

约 50% 的开发时间都花在阅读代码上；腐化的代码最终会迫使项目重写。

在可行时让函数保持纯粹——易于阅读和测试。
不要把可变对象传来传去以便就地填充（“装配式”代码）——应返回新的值。传递一个随后会被修改的可变对象，迫使读者同时打开两个文件才能理解一件事。
会修改输入的函数，其名称必须恰当。
优先选择平凡易懂的构造。 那个函数式 map-reduce 单行表达式虽然看着漂亮，但请老老实实写一个 4 行 for 循环。优先保证代码可调试，而非炫技。
一致性优于个人风格。 一个代码库不应成为 10 种风格拼凑的大杂烩。遵循周围的格式/命名/导入约定；标记任何破坏这些约定的改动。
良好的代码品味： 重新组织结构，让边缘情况成为常规情况，从而移除特殊分支。问自己：能否更简单？代码能否更少？是否变成了缩进过深的 if-else 汤？
优先扩展共享组件，而不是复制粘贴后各自演化。 同一件事出现 3–4 个分叉实现——长期发展速度会变慢。避免模块间紧密耦合；通过清晰、有文档的公共 API 集成。
为公共模块/类/函数编写 docstring；优先使用类型注解，而不是用文字描述类型（“类型提示好 10 倍”）；类型检查器能发现不易察觉的 bug。
Docstring 只应提及重要事项。保持简短直接。不要解释从函数名就能轻易理解的内容。专注于“为何”。
将不相关的改动拆分为独立的 commit/PR——这样能让审查保持聚焦，并使
```
git blame
```
/revert 清晰整洁。

6. API design & backward compatibility

6. API 设计与向后兼容性

Principle of least astonishment: an API's name + signature should convey ~90% of intent; users shouldn't be surprised by behavior.
Reject loose/overloaded parameters that accept many disjoint types (string/dict/list/None). Prefer separate single-purpose functions. Beware implicit fallbacks; use explicit variants. "APIs whose correct use depends on tribal knowledge are a liability."
Build for extension, not override. Provide hooks; never monkey-patch core at runtime ("inexcusably horrible" — breaks future fixes) and never copy a whole core file to change a few lines (fixes won't propagate).
Backward compatibility is an obligation for mature/public APIs. Follow semver; minor versions = zero breaking changes. Breaking changes include: removing public functions/fields, reordering args, new mandatory args, changed business logic, moved/renamed files (broken imports), bumped shared deps. Renaming without keeping the old name as an alias is an unnecessary break. Every breaking change ships a deprecation warning + docs.
Watch for schema breaking-change footguns: adding mandatory fields to existing sites, making long-lived fields unique (needs a data patch), changing field types without patches, removing fields.
Schema changes that silently skip existing sites need a data patch. Single doctypes don't sync new-field defaults to existing sites, and a field-type change (e.g. text→int) doesn't convert existing values — both need an explicit patch, tested against a populated site.
New parameters go last as keyword args with safe defaults (
```
None
```
, not
```
""
```
) so existing positional callers don't break. When renaming, keep the old name as a shim:
```
def old_name(...): return new_name(...)
```
.
Patch hygiene. Data patches must be idempotent (safe to re-run), correctly ordered (run after the field/doctype they read exists), and live in the right app (a framework change is patched in the framework, not the downstream app).
A modified existing test is a red flag. If making a change pass required editing an existing test's assertions, you've likely broken a real workflow — justify it explicitly rather than bending the test.

最小意外原则： API 的名称 + 签名应传达约 90% 的意图；用户不应对行为感到意外。
拒绝宽松/重载的参数，即允许许多互不相关的类型（字符串/字典/列表/None）。优先提供职责单一的独立函数。警惕隐式回退；使用明确的变体。“正确使用依赖口口相传知识的 API 是一种负担。”
为扩展而设计，而非为覆盖而设计。 提供 hook；绝不要在运行时 monkey-patch 核心（“无可辩解地糟糕”——会破坏未来修复），也绝不要复制整个核心文件只为改动几行（这样修复无法传播）。
向后兼容是成熟/公共 API 的义务。 遵循 semver；小版本 = 零破坏性变更。破坏性变更包括：移除公共函数/字段、调整参数顺序、新增必填参数、更改业务逻辑、移动/重命名文件（导致导入损坏）、升级共享依赖。重命名却不保留旧名称作为别名属于不必要的破坏。每项破坏性变更都应随附弃用警告 + 文档。
注意 schema 破坏性变更的陷阱： 向现有站点添加必填字段、将长期存在的字段设为 unique（需要 data patch）、不通过 patch 就更改字段类型、删除字段。
静默跳过现有站点的 schema 变更需要 data patch。 单个 doctype 不会把新字段默认值同步到现有站点，字段类型变更（例如 text→int）也不会转换现有值——两者都需要显式 patch，并针对包含真实数据的站点进行测试。
新参数放在最后，作为带安全默认值的 keyword args（
```
None
```
，而不是
```
""
```
），这样现有的位置参数调用方不会中断。重命名时，将旧名称保留为 shim：
```
def old_name(...): return new_name(...)
```
。
Patch 规范。 Data patch 必须幂等（可安全重复运行）、顺序正确（在其读取的字段/doctype 已存在之后运行），并位于正确的 app 中（框架变更应在框架中修补，而不是在下游 app 中修补）。
修改已有测试是一个危险信号。 如果为了让变更通过而不得不编辑已有测试的断言，你很可能破坏了真实业务流程——应明确说明理由，而不是扭曲测试。

7. Testing

7. 测试

Each PR needs decent test coverage — patch coverage on the diff, not just project coverage. (Frappe target: 85% covered lines in the diff.) Tests should capture the most-used business scenarios.
Regression test every fix. A bug fix without a test that would have caught it invites the regression back. For extreme-consequence (stateful/compliance) code, go beyond examples — property-based testing (Hypothesis).
Flag missing migration/data-patch coverage. Schema changes and data patches are the highest-risk, least-tested area; a change that alters fields or migrates data needs a patch tested against a realistic, populated site (empty tables always "migrate" successfully even when the change is invalid).
Tests must be deterministic and independent. No
```
random
```
(flaky); no reliance on state left by other tests (order-dependence); use
```
freeze_time
```
for time-dependent logic.

每个 PR 都需要足够的测试覆盖率——关注 diff 的覆盖率，而不只是项目覆盖率。（Frappe 的目标：diff 中 85% 的代码行被覆盖。）测试应覆盖最常用的业务场景。
为每项修复添加回归测试。 没有一个原本能够捕获该 bug 的测试，bug 修复就容易再次回归。对于后果极严重的（有状态/合规）代码，应更进一步——采用基于属性的测试（property-based testing，Hypothesis）。
标记缺失的 migration/data-patch 测试。 Schema 变更和 data patch 是风险最高、测试最少的领域；任何修改字段或迁移数据的变更，都需要 patch，并针对逼真的、包含数据的站点进行测试（空表总是能“成功迁移”，即使变更本身无效）。
测试必须确定且相互独立。 不使用
```
random
```
（会导致不稳定）；不依赖其他测试遗留的状态（顺序依赖）；对依赖时间的逻辑使用
```
freeze_time
```
。

8. Error messages, logging & observability

8. 错误消息、日志与可观测性

Error message quality is a legitimate review item. Titles must be specific and Google-able (never "Message"/"Error"). Reference field names as fields. State what changed: the row, the field, and before→after values (
```
1 → 2
```
). The user must know "qty changed from what to what?"
Surface failures to the affected party — "a broken email setup is the user's problem only if they know it's broken."
Log things. Preserve tracebacks/exception context (orders of magnitude easier debugging). Log destructive/admin actions with attributable identity (who, when, from where), persisted outside ephemeral containers.

错误消息质量是合理的审查项。 标题必须具体且便于 Google 搜索（绝不能只是“Message”/“Error”）。引用字段时，应明确指出字段名。说明发生了什么变化：具体行、字段以及变化前→变化后的值（
```
1 → 2
```
）。用户必须知道“数量从什么改成了什么？”
向受影响方暴露失败——“邮件配置损坏只有在用户知道它已损坏时，才是用户的问题。”
记录重要事项。 保留 traceback/异常上下文（会让调试容易几个数量级）。记录破坏性/管理操作，并附上可归因的身份信息（谁、何时、从何处），且将日志持久化在临时容器之外。