184 lines
4.8 KiB
Markdown
184 lines
4.8 KiB
Markdown
# Follow-up PRD：收敛 Home Admin 未登录重定向测试的实现耦合
|
||
|
||
## 背景
|
||
|
||
在验收 Home 相关改动时，存在一个**与本次功能无关的旧测试失败**：
|
||
|
||
- `home/tests/test_admin_permissions.py`
|
||
- `test_admin_redirect_uses_public_home_url_when_unauthenticated`
|
||
|
||
该测试失败并不表明 Home Admin 未登录跳转功能失效，而是因为测试对 redirect URL 的**完整字符串**做了过度刚性的断言，和当前共享登录守卫 / 共享服务 URL 逻辑产生了耦合漂移。
|
||
|
||
---
|
||
|
||
## 问题定义
|
||
|
||
当前测试断言类似：
|
||
|
||
```python
|
||
assert query["redirect"][0] == encode_redirect("http://127.0.0.1:8000/admin?tab=drafts")
|
||
assert decode_redirect(query["redirect"][0]) == "http://127.0.0.1:8000/admin?tab=drafts"
|
||
```
|
||
|
||
这类断言的问题在于：
|
||
|
||
1. **把具体实现细节写死了**
|
||
- host 必须是 `127.0.0.1`
|
||
- scheme 必须是 `http`
|
||
- 端口必须是 `8000`
|
||
- 完整 URL 字符串必须完全一致
|
||
|
||
2. **与共享 URL 生成逻辑耦合过深**
|
||
- 当前服务 URL 由共享模块生成
|
||
- 未来若 `SERVICE_DEV_HOST`、`SERVICE_DEV_SCHEME`、base_url 或 admin guard 逻辑调整，测试会失败
|
||
- 但真实业务行为可能仍然正确
|
||
|
||
3. **测试的重点偏了**
|
||
- 它本应验证“未登录会跳到登录页，并保留回跳目标”
|
||
- 现在却变成了验证“当前内部 URL 拼接实现恰好等于某个历史字符串”
|
||
|
||
---
|
||
|
||
## 目标
|
||
|
||
将该测试从“验证固定实现细节”调整为“验证稳定行为契约”，使其：
|
||
|
||
- 能正确覆盖未登录重定向语义
|
||
- 不再因共享 URL 生成细节微调而误报失败
|
||
- 不改变现有业务逻辑，只修测试
|
||
|
||
---
|
||
|
||
## 方案
|
||
|
||
## 一、保留要测的核心行为
|
||
|
||
该测试真正应该验证的是：
|
||
|
||
1. 未登录访问 `/admin?tab=drafts`
|
||
2. 返回 `302`
|
||
3. 重定向目标是登录页
|
||
4. 登录页 URL 中包含 `redirect` 参数
|
||
5. `redirect` 解码后仍然指向 Home Admin
|
||
6. 原始查询参数 `tab=drafts` 被保留
|
||
|
||
这才是稳定契约。
|
||
|
||
---
|
||
|
||
## 二、去掉对完整绝对 URL 字符串的硬编码
|
||
|
||
### 不建议继续这样断言
|
||
|
||
```python
|
||
assert decode_redirect(query["redirect"][0]) == "http://127.0.0.1:8000/admin?tab=drafts"
|
||
```
|
||
|
||
原因：
|
||
- 这会把测试绑死在当前某个局部实现上
|
||
- 不利于 shared admin guard / shared ports 的后续演进
|
||
|
||
### 建议改为结构化断言
|
||
|
||
先解码：
|
||
|
||
```python
|
||
redirect_target = decode_redirect(query["redirect"][0])
|
||
parsed_target = urlparse(redirect_target)
|
||
parsed_query = parse_qs(parsed_target.query)
|
||
```
|
||
|
||
然后断言：
|
||
|
||
```python
|
||
assert parsed_target.path == "/admin"
|
||
assert parsed_query.get("tab") == ["drafts"]
|
||
```
|
||
|
||
### 登录页断言仍然保留
|
||
|
||
```python
|
||
assert response.status_code == 302
|
||
parsed = urlparse(response.headers["location"])
|
||
assert parsed.path == "/login"
|
||
assert "redirect" in parse_qs(parsed.query)
|
||
```
|
||
|
||
---
|
||
|
||
## 三、如果确实要校验 host / scheme，必须改成从共享配置推导
|
||
|
||
如果团队认为还需要校验回跳目标的 host/scheme，也不要手写：
|
||
|
||
- `http://127.0.0.1:8000`
|
||
|
||
而应从共享配置推导，例如：
|
||
- `get_service_url("home", is_dev=...)`
|
||
|
||
这样至少保证：
|
||
- 测试与当前规范保持一致
|
||
- 不会因重复硬编码造成漂移
|
||
|
||
但从测试稳定性角度看，**本次建议优先只验证 path + query**，不要把 host 作为必须项。
|
||
|
||
---
|
||
|
||
## 修改范围
|
||
|
||
### 必改文件
|
||
- `home/tests/test_admin_permissions.py`
|
||
|
||
### 修改对象
|
||
- `test_admin_redirect_uses_public_home_url_when_unauthenticated`
|
||
|
||
### 不改范围
|
||
本次 follow-up **不修改**：
|
||
- `home/src/routes/admin.py`
|
||
- `shared/core/ports.py`
|
||
- `shared admin guard`
|
||
- 任何业务重定向逻辑
|
||
|
||
这是一个**纯测试修正 PRD**。
|
||
|
||
---
|
||
|
||
## 验收标准
|
||
|
||
修正后，该测试应满足：
|
||
|
||
1. 未登录访问 `/admin?tab=drafts` 返回 `302`
|
||
2. location 指向 `/login`
|
||
3. query 中包含 `redirect`
|
||
4. `redirect` 解码后：
|
||
- path 是 `/admin`
|
||
- query 中 `tab == drafts`
|
||
5. 不再对完整固定 URL 字符串做硬编码断言
|
||
|
||
---
|
||
|
||
## 风险控制
|
||
|
||
### 为什么不能直接删掉测试
|
||
|
||
因为这个测试本身验证的场景仍然有价值：
|
||
- 未登录状态下，admin 页面必须正确跳转到登录页
|
||
- 登录后回跳目标必须保留
|
||
|
||
问题不在“要不要测”，而在“测法不对”。
|
||
|
||
### 为什么不建议顺手改业务实现
|
||
|
||
当前没有证据表明业务重定向逻辑是坏的。
|
||
失败来自测试与实现耦合过深，而不是功能错误。
|
||
|
||
因此本次 follow-up 应坚持：
|
||
- **只修测试，不修业务逻辑**
|
||
|
||
---
|
||
|
||
## 结论
|
||
|
||
这个 follow-up 的本质，是把一个**脆弱测试**改成一个**验证真实契约的稳定测试**。
|
||
|
||
目标不是让测试“勉强通过”，而是让它以后在共享登录守卫、共享 URL 配置继续演进时，仍然只对真正的行为回归报警。
|