Test failures in git-repository from_bare_parent_repo from_nonbare_parent_repo

[joelparkerhenderson]

Duplicates

• I have searched the existing issues

Current behavior 😯

I'm trying git-repository for the first time, and seeing test failures:

$ cd git-repository 
$ cargo test

Output:

failures:

---- repository::worktree::from_bare_parent_repo stdout ----
thread 'repository::worktree::from_bare_parent_repo' panicked at 'called `Result::unwrap()` on an `Err` value: ParseIntError { kind: InvalidDigit }', tests/tools/src/lib.rs:69:79
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- repository::worktree::from_nonbare_parent_repo stdout ----
thread 'repository::worktree::from_nonbare_parent_repo' panicked at 'Lazy instance has previously been poisoned', /Users/jph/.cargo/registry/src/github.com-1ecc6299db9ec823/once_cell-1.13.0/src/lib.rs:1231:25


failures:
    repository::worktree::from_bare_parent_repo
    repository::worktree::from_nonbare_parent_repo

test result: FAILED. 90 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.69s

error: test failed, to rerun pass '--test git'

Expected behavior 🤔

Expected: tests all pass

Steps to reproduce 🕹

Reproduce:

$ cd git-repository 
$ cargo test

I'm using current macOS on M1:

$ uname -a
Darwin … 21.6.0 Darwin Kernel Version 
21.6.0: Wed Aug 10 14:28:23 PDT 2022; 
root:xnu-8020.141.5~2/RELEASE_ARM64_T6000 arm64

[Byron]

Thanks for letting me know, I will take a look. Could you also post your git --version here? Thanks.

[joelparkerhenderson]

Yes thanks:

$ git --version
git version 2.37.2

[Byron]

Thanks! It's strange though, I had expected something more special, after all it appears to be unable to interpret 2.37.2 as numbers.

I have beefed up the error reporting (or so I hope) in main, could you retry again?

[joelparkerhenderson]

Edit: I think I found the issue and fixed it. See #511

Yes thank you. I did a typical git pull, then tyipcal cargo test.

failures:

---- repository::worktree::from_bare_parent_repo stdout ----
thread 'repository::worktree::from_bare_parent_repo' panicked at 'git version to be parsable: "Could not parse version from output of 'git --version' (\"git version 2.37.2\\n\") with error: invalid digit found in string"', tests/tools/src/lib.rs:69:79
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- repository::worktree::from_nonbare_parent_repo stdout ----
thread 'repository::worktree::from_nonbare_parent_repo' panicked at 'Lazy instance has previously been poisoned', /Users/jph/.cargo/registry/src/github.com-1ecc6299db9ec823/once_cell-1.13.0/src/lib.rs:1231:25


failures:
    repository::worktree::from_bare_parent_repo
    repository::worktree::from_nonbare_parent_repo

test result: FAILED. 90 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.14s

error: test failed, to rerun pass '--test git'

[joelparkerhenderson]

See #511

See this function:

fn git_version_from_bytes(bytes: &[u8]) -> Result<(u8, u8, u8)> {

Possibly this line...

.split(|b| *b == b' ')

...should be changed to something like:

.split(|b| *b == b' ' || *b == b'\n')

Or even better changed to the logic of "where b is not digit and not period".

And also a new test case for output that has a trailing newline?

Some defensive code could help perhaps, such as moving the three numbers out of the tuple and into a step-by-step assignment, such as:

let major: u8 = numbers.next().expect("major")?;
let minor: u8 = numbers.next().expect("minor")?;
let patch: u8 = numbers.next().expect("patch")?;

[Byron]

Great, thanks a lot for the fix!

I think it's alright to keep it as simple as possible and wait for the improved error handling to tell us about other ways of git reporting its version so parsing can be improved as needed.

Thanks again for your involvement, much appreciated.