Skip the SHA256 test if git has not support for it #155
Conversation
If you are building and running the tests in an environment with an older version of git, it might not have SHA256 support. This should not cause the git-sizer test suite to fail as it's not an issue with git-sizer. Detect this situation and skip the test.
There was a problem hiding this comment.
Pull request overview
This PR adds logic to gracefully skip the SHA256 test when running in environments with older Git versions that lack SHA256 support. This prevents test suite failures due to missing Git features rather than issues with git-sizer itself.
Changes:
- Modified the TestSHA256 function to capture git command output
- Added detection for the "unknown option" error indicating lack of SHA256 support
- Skip the test with an informative message when SHA256 is not available
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err != nil && strings.HasPrefix(string(output), "error: unknown option `object-format'") { | ||
| t.Skip("skipping due to lack of SHA256 support") | ||
| } | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
If the git init command fails for a reason other than missing SHA256 support, the error message at line 875 won't include the command output, making it harder to diagnose the failure. Consider adding the output to the error message, for example by using require.NoError(t, err, \"git init failed: %s\", string(output)) or a similar approach that includes the output in failure cases.
| require.NoError(t, err) | |
| require.NoError(t, err, "git init failed: %s", string(output)) |
git_sizer_test.go
Outdated
| if err != nil && strings.HasPrefix(string(output), "error: unknown option `object-format'") { | ||
| t.Skip("skipping due to lack of SHA256 support") |
There was a problem hiding this comment.
The error message matching uses an exact prefix match which may be fragile across different git versions or locales. Consider using strings.Contains to match a key part of the error (e.g., "unknown option" and "object-format") for more robust detection, or document the git version assumptions if this exact format is intentional.
| if err != nil && strings.HasPrefix(string(output), "error: unknown option `object-format'") { | |
| t.Skip("skipping due to lack of SHA256 support") | |
| if err != nil { | |
| msg := string(output) | |
| if strings.Contains(msg, "unknown option") && strings.Contains(msg, "object-format") { | |
| t.Skip("skipping due to lack of SHA256 support") | |
| } |
Version 2 wants to use the old URL so that also fails to run. The latest is version 6 so let's update to that and at the same time update to the same Go version that we want to download in the build script.
mhagger
left a comment
mhagger
left a comment
There was a problem hiding this comment.
Copilot's second suggestion sounds like a good idea, but with or without that change, 👍
|
The locale comment doesn't make much sense if we're still putting in the "unknown option" text. We can presumably reduce it to "object-format" if we're not trying to match exactly. |
As pointed out by the robot, this can be an issue with different locales. It is enough for our purposes to know that the error message includes "object-format" so we know it's unhappy with it.
If you are building and running the tests in an environment with an older version of git, it might not have SHA256 support. This should not cause the git-sizer test suite to fail as it's not an issue with git-sizer.
Detect this situation and skip the test.