From a316a5481a6d143ca190e9fd19e238af49c51e2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 15 Oct 2026 12:37:11 -0400 Subject: [PATCH] R6.6 round 2 fix: Git lookup tests use only temp folders The lookup is split from the developer fallback, so the tests need no built Git and check NoGit explicitly. Shorter stage.sh comment. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/ely-platform/src/git.rs | 55 ++++++++++++++++++++++++---------- packaging/stage.sh | 2 +- tasks/reviews/R6.6-r2.md | 5 ++++ 3 files changed, 45 insertions(+), 17 deletions(-) create mode 100644 tasks/reviews/R6.6-r2.md diff --git a/crates/ely-platform/src/git.rs b/crates/ely-platform/src/git.rs index 7b53333..9a3184a 100644 --- a/crates/ely-platform/src/git.rs +++ b/crates/ely-platform/src/git.rs @@ -81,10 +81,15 @@ fn own(exe: &Path) -> io::Result { // ElyTree carries Git; the other apps use its copy. let trees = crate::apps::beside(exe, AppId::Tree).into_iter().chain(crate::apps::registered(AppId::Tree)); let built = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../engines/git").join(GIT); + find(exe, trees, built) +} + +/// The first Git beside `exe`, beside a tree, or built. +fn find(exe: &Path, trees: impl IntoIterator, built: PathBuf) -> io::Result { exe.parent() .map(Path::to_path_buf) .into_iter() - .chain(trees.filter_map(|tree| tree.parent().map(Path::to_path_buf))) + .chain(trees.into_iter().filter_map(|tree| tree.parent().map(Path::to_path_buf))) .flat_map(|dir| [dir.join("git").join(GIT), dir.join("../Resources/git").join(GIT)]) .chain([built]) .find(|path| path.is_file()) @@ -130,7 +135,18 @@ mod tests { assert_eq!(super::parse_version("hello"), None); } - use super::{GIT, own}; + use super::{GIT, NoGit, find}; + use crate::{AppId, apps::beside}; + use std::path::Path; + + /// `find` with only temp folders: no installed or built Git. + fn found(exe: &Path, root: &Path) -> std::io::Result { + find(exe, beside(exe, AppId::Tree), root.join("built").join(GIT)) + } + + fn same(found: std::path::PathBuf, git: &Path) { + assert_eq!(std::fs::canonicalize(found).expect("found"), std::fs::canonicalize(git).expect("git")); + } #[test] fn apps_beside_elytree_use_its_git() { @@ -140,25 +156,32 @@ mod tests { std::fs::create_dir_all(&mark).expect("ElyMark"); std::fs::create_dir_all(apps.path().join("ElyTree.app/Contents/MacOS")).expect("ElyTree"); let exe = mark.join("ely-mark"); - let built = own(&exe).expect("the built Git"); - assert!(built.ends_with(std::path::Path::new("engines/git").join(GIT))); + let missing = found(&exe, apps.path()).expect_err("no Git yet"); + assert!(missing.get_ref().is_some_and(|error| error.is::()), "{missing}"); std::fs::create_dir_all(git.parent().expect("a folder")).expect("ElyTree's Git"); std::fs::write(&git, "").expect("git"); - let found = own(&exe).expect("ElyTree's Git"); - assert_eq!(std::fs::canonicalize(found).expect("found"), std::fs::canonicalize(&git).expect("git")); + same(found(&exe, apps.path()).expect("ElyTree's Git"), &git); } #[test] fn a_windows_install_uses_elytrees_git() { - for (mark, tree) in [("Ely Office/ElyMark", "Ely Office/ElyTree")] { - let root = tempfile::tempdir().expect("a temp dir"); - let exe = root.path().join(mark).join("ely-mark"); - let git = root.path().join(tree).join("git").join(GIT); - std::fs::create_dir_all(exe.parent().expect("a folder")).expect("ElyMark"); - std::fs::create_dir_all(git.parent().expect("a folder")).expect("ElyTree's Git"); - std::fs::write(&git, "").expect("git"); - let found = std::fs::canonicalize(own(&exe).expect("ElyTree's Git")).expect("found"); - assert_eq!(found, std::fs::canonicalize(&git).expect("git"), "{mark}"); - } + let root = tempfile::tempdir().expect("a temp dir"); + let exe = root.path().join("Ely Office/ElyMark/ely-mark"); + let git = root.path().join("Ely Office/ElyTree/git").join(GIT); + std::fs::create_dir_all(exe.parent().expect("a folder")).expect("ElyMark"); + assert!(found(&exe, root.path()).is_err(), "no Git yet"); + std::fs::create_dir_all(git.parent().expect("a folder")).expect("ElyTree's Git"); + std::fs::write(&git, "").expect("git"); + same(found(&exe, root.path()).expect("ElyTree's Git"), &git); + } + + #[test] + fn the_built_git_comes_last() { + let root = tempfile::tempdir().expect("a temp dir"); + let exe = root.path().join("app/ely-mark"); + let built = root.path().join("built").join(GIT); + std::fs::create_dir_all(built.parent().expect("a folder")).expect("built"); + std::fs::write(&built, "").expect("git"); + same(found(&exe, root.path()).expect("the built Git"), &built); } } diff --git a/packaging/stage.sh b/packaging/stage.sh index ca97bfc..11702e4 100755 --- a/packaging/stage.sh +++ b/packaging/stage.sh @@ -64,7 +64,7 @@ if [ "$slug" = ely-pdf ]; then mkdir "$out/tessdata" cp "$tessdata"/{eng,chi_sim,chi_tra}.traineddata "$out/tessdata/" fi -# ElyTree carries Git; on Linux, whose AppImage stands alone, ElyMark too. +# ElyTree carries Git; so does Linux ElyMark's standalone AppImage. if [ "$slug" = ely-tree ] || { [ "$slug" = ely-mark ] && [ "$(uname)" = Linux ]; }; then need "$root/engines/git" "scripts/build-git.sh and scripts/fetch-git-lfs.sh" cp -R "$root/engines/git" "$out/git" diff --git a/tasks/reviews/R6.6-r2.md b/tasks/reviews/R6.6-r2.md new file mode 100644 index 0000000..248a686 --- /dev/null +++ b/tasks/reviews/R6.6-r2.md @@ -0,0 +1,5 @@ +FAIL + +- crates/ely-platform/src/git.rs:143 — The revised sibling-bundle test now requires an actual development Git before creating its Git fixture. In a clean checkout without the ignored `engines/git` directory, `cargo test --locked -p ely-platform --lib git::tests -- --test-threads=1` panics at `expect("the built Git")` with `NotFound/NoGit` on both Windows and Linux. Independent builds of 8580aed reproduce this (`.work/R6.6-r2/windows-clean-git-tests.log`, `linux-clean-git-tests.log`); supplying a built-Git fixture makes the test pass. Make the sibling lookup test independent of developer engine artifacts and cover the missing-provider result explicitly. + +- packaging/stage.sh:67 — The new comment contains 11 words, exceeding agents.md's ten-word limit. Running the repository comment checker against only this change's six source/script files exits 1 on this line (`.work/R6.6-r2/scoped-comments.log`). Shorten the comment to ten words or fewer.