Skip to content

test/2024-06-08-connect-remote-shims #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ackkerman merged 2 commits into from
Jul 23, 2025
Merged

test/2024-06-08-connect-remote-shims #22

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c935ac2
test: check connect remote update paths
ackkerman Jul 23, 2025
f374e79
Merge branch 'main' into 2025-07-23-add-tests-for-git-remote-command
ackkerman Jul 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 74 additions & 2 deletions tests/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,57 @@ fi
shim
}

/// `git remote get-url` を失敗させて remote add を強制させるシム
fn fake_git_remote_missing(dir: &tempfile::TempDir) -> std::path::PathBuf {
let shim = dir.path().join("git");
fs::write(
&shim,
r#"#!/usr/bin/env sh
if [ "$1" = "config" ]; then
/usr/bin/git "$@"
Copy link

@gemini-code-assist gemini-code-assist bot Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The path to the git executable is hardcoded as /usr/bin/git. This can make the tests less portable if git is installed in a different location on other systems. Consider dynamically finding the path to the git executable during test setup and injecting it into the shim script.

elif [ "$1" = "remote" ] && [ "$2" = "get-url" ]; then
exit 1
else
echo git "$@"
exit 0
fi
"#,
)
.unwrap();
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
fs::set_permissions(&shim, fs::Permissions::from_mode(0o755)).unwrap();
}
shim
}

/// `git remote get-url` を成功させつつ異なる URL を返し、set-url を誘発するシム
fn fake_git_remote_mismatch(dir: &tempfile::TempDir) -> std::path::PathBuf {
let shim = dir.path().join("git");
fs::write(
&shim,
r#"#!/usr/bin/env sh
if [ "$1" = "config" ]; then
/usr/bin/git "$@"
Copy link

@gemini-code-assist gemini-code-assist bot Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to line 68, the path to the git executable is hardcoded as /usr/bin/git. Consider dynamically finding the path to the git executable during test setup and injecting it into the shim script to improve portability.

elif [ "$1" = "remote" ] && [ "$2" = "get-url" ]; then
echo https://example.com/other.git
exit 0
else
echo git "$@"
exit 0
fi
"#,
)
.unwrap();
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
fs::set_permissions(&shim, fs::Permissions::from_mode(0o755)).unwrap();
}
shim
Comment on lines +62 to +109
Copy link

@gemini-code-assist gemini-code-assist bot Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There's significant code duplication between fake_git_remote_missing and fake_git_remote_mismatch. The logic for creating the file, writing the script content, and setting permissions is nearly identical. To improve maintainability and reduce redundancy, consider extracting this repeated logic into a single helper function that accepts the script content as an argument.

}

/// `git config` が失敗するシム
fn fake_git_fail_config(dir: &tempfile::TempDir) -> std::path::PathBuf {
let shim = dir.path().join("git");
Expand Down Expand Up @@ -217,6 +268,27 @@ fn connect_fails_on_git_config_error() {
std::env::var("PATH").unwrap()
);

Command::cargo_bin("gh-sync")
.unwrap()
.current_dir(repo.path())
.env("PATH", &path_env)
.args(&["connect", "web", "git@github.com:a/b.git"])
.assert()
.success()
.stdout(predicate::str::contains("git remote add"));
}

#[test]
fn connect_updates_remote_url() {
let repo = setup_repo();
let git_shim = fake_git_remote_mismatch(&repo);

let path_env = format!(
"{}:{}",
git_shim.parent().unwrap().display(),
std::env::var("PATH").unwrap()
);
Comment on lines +287 to +290
Copy link

@gemini-code-assist gemini-code-assist bot Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The PATH variable is constructed using a hardcoded colon (:), which is not portable. Use std::env::consts::PATH_SEPARATOR. Also, unwrap() on an environment variable can lead to fragile tests. Use expect() with a clear error message to improve diagnostics if the test fails.

Suggested change
"{}:{}",
git_shim.parent().unwrap().display(),
std::env::var("PATH").unwrap()
);
let path_env = format!(
"{}{}{}",
git_shim.parent().unwrap().display(),
std::env::consts::PATH_SEPARATOR,
std::env::var("PATH").expect("PATH environment variable must be set")
);


Command::cargo_bin("gh-sync")
.unwrap()
.current_dir(repo.path())
Expand All @@ -228,9 +300,9 @@ fn connect_fails_on_git_config_error() {
}

#[test]
fn connect_fails_on_git_config_error() {
fn connect_adds_remote_when_missing() {
let repo = setup_repo();
let git_shim = fake_git_fail_config(&repo);
let git_shim = fake_git_remote_missing(&repo);

let path_env = format!(
"{}:{}",
Expand Down
Loading