Skip to content

Add git compatibility tests to CI (#436)#457

Closed
Krishnachaitanyakc wants to merge 11 commits intogit-ai-project:mainfrom
Krishnachaitanyakc:add-git-bash-tests-ci
Closed

Add git compatibility tests to CI (#436)#457
Krishnachaitanyakc wants to merge 11 commits intogit-ai-project:mainfrom
Krishnachaitanyakc:add-git-bash-tests-ci

Conversation

@Krishnachaitanyakc
Copy link
Copy Markdown
Contributor

@Krishnachaitanyakc Krishnachaitanyakc commented Feb 4, 2026

Add a GitHub Actions workflow that runs a subset of the official git test suite with git-ai enabled to ensure core git functionality is not broken by git-ai changes.

Changes:

  • Add .github/workflows/git-compat-tests.yml workflow that:

    • Clones official git repo (pinned to v2.47.1)
    • Builds git-ai and sets up gitwrap symlinks
    • Runs ~150 core git tests with both standard git and git-ai
    • Compares results to detect regressions (git-ai-only failures)
    • Uploads test results as artifacts
  • Add tests/git-compat/compare_results.py for CI result comparison

  • Add tests/git-compat/README.md with documentation for local development and debugging

Test coverage includes: basic operations, branching, merging, rebase, cherry-pick, stash, diff, log, remote operations, hooks, notes, reset, checkout, and more.

Closes #436


Open with Devin

Add a GitHub Actions workflow that runs a subset of the official git
test suite with git-ai enabled to ensure core git functionality is
not broken by git-ai changes.

Changes:
- Add .github/workflows/git-compat-tests.yml workflow that:
  - Clones official git repo (pinned to v2.47.1)
  - Builds git-ai and sets up gitwrap symlinks
  - Runs ~150 core git tests with both standard git and git-ai
  - Compares results to detect regressions (git-ai-only failures)
  - Uploads test results as artifacts

- Add tests/git-compat/compare_results.py for CI result comparison

- Add tests/git-compat/README.md with documentation for local
  development and debugging

Test coverage includes: basic operations, branching, merging, rebase,
cherry-pick, stash, diff, log, remote operations, hooks, notes, reset,
checkout, and more.

Closes git-ai-project#436
@svarlamov
Copy link
Copy Markdown
Member

@Krishnachaitanyakc Do you have any thoughts on the failures?

- Add GIT_TEST_INSTALLED check to bypass all hooks when running in
  the official git test suite, avoiding test isolation pollution
- Add GIT_AI_DISABLED and GIT_AI_PASSTHROUGH environment variables
  to allow users to explicitly bypass git-ai in CI or scripts
- Make RepoStorage initialization lazy - directories are now created
  only when write operations are performed, not on every command
- Update working_log_for_base_commit() and append_rewrite_event()
  to call ensure_initialized() before writing

This fixes regressions in the git compatibility tests where git-ai
was creating .git/ai/ directories and other side effects during
read-only operations.
@Krishnachaitanyakc
Copy link
Copy Markdown
Contributor Author

Krishnachaitanyakc commented Feb 4, 2026

Put up a fix for them. Will check the results soon, they are passing locally

- Add GIT_TEST_INSTALLED check to bypass all hooks when running in
  the official git test suite, avoiding test isolation pollution
- Add GIT_AI_DISABLED and GIT_AI_PASSTHROUGH environment variables
  to allow users to explicitly bypass git-ai in CI or scripts
- Make RepoStorage initialization lazy - directories are now created
  only when write operations are performed, not on every command
- Update working_log_for_base_commit() and append_rewrite_event()
  to call ensure_initialized() before writing

This fixes regressions in the git compatibility tests where git-ai
was creating .git/ai/ directories and other side effects during
read-only operations.
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

The tests were still expecting for_repo_path to eagerly create
directories, but it was changed to lazy initialization. Update
tests to call ensure_initialized() before asserting directory
existence or writing files.
@Krishnachaitanyakc
Copy link
Copy Markdown
Contributor Author

My IDE is behaving odd, some changes were missing from my last commit, pushing them

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

t9300-fast-import.sh \
t9350-fast-export.sh \
2>&1 | tee standard_test_results.txt
echo "standard_exit_code=$?" >> $GITHUB_OUTPUT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Exit code capture captures tee's exit code instead of prove's

The workflow captures exit codes using $? after piping to tee, which captures tee's exit code (typically 0) rather than prove's actual exit code.

Root Cause

In bash, when using a pipeline like command 2>&1 | tee file, the $? variable captures the exit status of the last command in the pipeline (tee), not the first command (prove). Since tee almost always succeeds, standard_exit_code and gitai_exit_code will typically be 0 even when tests fail.

To capture the correct exit code, you would need to use set -o pipefail or use bash's PIPESTATUS array: ${PIPESTATUS[0]}.

Impact: Currently minimal since these exit codes are written to $GITHUB_OUTPUT but never used - the comparison script parses the actual test output files instead. However, this could confuse developers debugging the workflow or if these outputs are used in the future.

Prompt for agents
To fix the exit code capture in both test steps, add `set -o pipefail` at the beginning of the run script or use PIPESTATUS. For the standard git tests step (lines 84-259), modify line 84 to start with `set -o pipefail` before the GIT_TEST_INSTALLED line. Similarly for the git-ai tests step (lines 265-440), add `set -o pipefail` at line 265. This ensures that `$?` captures the exit code of `prove` rather than `tee`. Note: Since these exit codes are not currently used in the workflow, this is a low-priority fix.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

This function was added but never called - all passthrough paths
use proxy_to_git instead, which supports post-command hooks and
signal handling.
@Krishnachaitanyakc
Copy link
Copy Markdown
Contributor Author

@svarlamov can you review

@svarlamov
Copy link
Copy Markdown
Member

Thanks for putting this together!

However, I think there's a logic issue that makes the current workflow a no-op. The PR adds a
check in handle_git() that bypasses all git-ai logic when GIT_TEST_INSTALLED is set — but
GIT_TEST_INSTALLED is exactly the env var the git test harness sets when it runs. So the flow is:

  1. CI sets GIT_TEST_INSTALLED=~/.git-ai-test/gitwrap/bin
  2. Git test harness calls the git symlink → git-ai
  3. git-ai sees GIT_TEST_INSTALLED, skips all its logic, and proxies directly to the real system
    git
  4. The "git-ai" run and the "standard git" run are functionally identical

This means the comparison will always show zero regressions regardless of what git-ai does, since
git-ai's own logic (hooks, repo storage, attributions, etc.) is never exercised.

The GIT_TEST_INSTALLED check in handle_git() was causing git-ai to
skip all its logic during the compatibility test suite, making both
the standard-git and git-ai runs identical. Remove the bypass so
git-ai hooks actually execute during tests. Suppress observability
disk writes in test mode to avoid polluting $HOME with telemetry.
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 10 additional findings in Devin Review.

Open in Devin Review

Comment thread tests/git-compat/compare_results.py Outdated
When a test crashes, times out, or produces no TAP output under git-ai,
it appears in prove's Test Summary Report with no Failed tests line,
leaving its failure set empty. The set difference (empty - empty) is
always empty, so the regression was silently missed. Detect these
complete failures by checking for tests in git-ai failures that don't
appear in standard failures at all.
…T_TRACE*

Add GIT_TEST_INSTALLED to skip_hooks check so authorship hooks don't
create refs/notes/ai during the git test suite. Strip GIT_TRACE* env
vars from internal exec_git calls to prevent duplicated trace output
that was causing t1510-repo-setup failures.
…dard subtest failures

When a test completely crashes under git-ai (empty failure set) but has
specific subtest failures under standard git, the old logic fell through
to the set-difference path: set() - {3, 7} = set(), silently dropping
the regression. Now any complete git-ai failure is flagged unless the
test also completely failed under standard git.
@svarlamov
Copy link
Copy Markdown
Member

Hey @Krishnachaitanyakc, I hacked out something pretty minimal for git compat today. Were there any bugs you found while trying to get the git compat tests going that might be worth fixing in future PRs?

@svarlamov svarlamov closed this Feb 9, 2026
@Krishnachaitanyakc
Copy link
Copy Markdown
Contributor Author

@svarlamov I haven't noticed any bugs actually, will keep an eye out in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add subset of git bash tests into our CI

2 participants