feat: add python support and default rs,py#1
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d0cb2c012
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for path in paths { | ||
| if detect_language(&path).is_none() { | ||
| continue; | ||
| } | ||
|
|
||
| sources.push((path.clone(), read_source(&path)?)); |
There was a problem hiding this comment.
load_sources() now reads any path that detect_language() recognizes, but build_whole_test_paths() only hands those buffers to the Rust and Python collectors. As a result, default/test-filtered runs eagerly open every *.js/*.ts file in the repo and decode it as UTF-8 even for --lang py or when there are no frontend changes at all. In repos with vendored/generated non-UTF8 JS/TS files this turns an otherwise valid diff into a hard error, and even with UTF-8 it adds a noticeable whole-repo I/O regression compared with the previous Rust-only loaders.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Scoped test-filter source loading to the requested --lang set in 69a9351, so --lang py no longer opens unrelated Rust/JS/TS buffers while building whole-test-path sets. Added explicit_python_lang_skips_loading_unselected_rust_sources to lock the non-UTF8 regression case.
| "function_definition" => Ok(extract_name(node, source)? | ||
| .map(|name| name.starts_with("test_")) | ||
| .unwrap_or(false)), |
There was a problem hiding this comment.
def test_* helpers from Python test regions
Because collect_regions() walks every descendant node, this branch classifies any function_definition named test_* as test code, including nested local helpers inside production functions. Pytest-style discovery does not collect those local defs, so edits to code like def build():\n def test_case(...): ... will be dropped from the default non-test stats and incorrectly reported under --test instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Changed Python test-region detection in 69a9351 to only treat top-level def test_* and top-level class Test* definitions as test regions. That excludes nested/local helpers and methods on non-test classes. Added ignores_test_named_methods_on_non_test_classes as a regression test for the misclassification path.
|
@codex review 用中文 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69a9351872
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn change_language(change: &FileChange) -> Option<&'static str> { | ||
| detect_language(&change.new_path).or_else(|| detect_language(&change.old_path)) |
There was a problem hiding this comment.
当一次 diff 被 Git 识别为跨语言重命名时（例如把 tests/test_mod.py 重命名成 src/lib.rs 并同时改几行），这里会只按 new_path/old_path 选出一个语言，然后把整份补丁都交给对应后端。结果旧侧的 Python 测试删除会按 Rust 规则分类，--test/--no-test 的增删统计都会失真；如果另一侧扩展名不再匹配，还可能直接跳过这条变更。这个问题是 Python 支持引入后的新输入类型，建议按 old/new 两侧分别分类，或把跨语言重命名退化为 delete+add。
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0bcb09f. Cross-language renames now classify old and new sides separately with the matching language backend, so tests/test_mod.py => src/lib.rs keeps the Python-side deletions under --test and --no-test. Added regressions in tests/lang_filter.rs and tests/cli_smoke.rs to lock both the language filter and end-to-end counting path.
| pub last: bool, | ||
|
|
||
| #[arg(long, value_name = "LANGS", default_value = "rs")] | ||
| #[arg(long, value_name = "LANGS", default_value = "rs,py")] |
There was a problem hiding this comment.
把默认语言改成 rs,py 后，普通的 git diff-stat 现在会进入 build_whole_test_paths() 去读取仓库里的全部 Python 文件，即使这次 diff 只有 Rust 变更。这样在混合仓库里只要存在一个合法但非 UTF-8 的 *.py（例如带编码声明的 legacy 文件），默认命令就会因为 read_to_string/git show 的 UTF-8 解码失败而直接报错；以前默认只处理 Rust 时不会触发这类 Python 读取。这里至少应只为本次变更里实际出现的语言加载源码。
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bcb09f57c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| detect_language(&change.old_path) | ||
| .into_iter() | ||
| .chain(detect_language(&change.new_path)) | ||
| .any(|language| requested.contains(&language)) |
There was a problem hiding this comment.
Matching on both old_path and new_path keeps cross-language renames in the filtered set, but the TestFilterMode::All path in main.rs still renders the full change.added/change.deleted. For a rename like tests/test_mod.py => src/lib.rs, git diff-stat --last --lang py --no-test-filter will therefore report the Rust insertions as part of the Python stats (and --lang rs will report the Python deletions). That makes language-scoped output inaccurate whenever a file changes extensions across the diff.
Useful? React with 👍 / 👎.
| if !should_load_source(&path, langs) { | ||
| continue; | ||
| } | ||
|
|
||
| sources.push((path.clone(), read_source(&path)?)); |
There was a problem hiding this comment.
This eagerly reads every selected-language file before whole-file test detection. For Python that is unnecessary—collect_python_whole_test_paths only looks at the path—so default --no-test / --test now fail if the repo contains any unrelated non-UTF8 or alternate-encoded *.py file. A single Latin-1 Python source elsewhere in the tree is enough to make git diff-stat abort before it reaches the changed file.
Useful? React with 👍 / 👎.
1 participant
Summary
rstors,pyand update docs/help textTest Plan