Skip to content

Fix: branch list empty#90

Closed
Meldiron wants to merge 10 commits into
mainfrom
fix-gitea-branch-list
Closed

Fix: branch list empty#90
Meldiron wants to merge 10 commits into
mainfrom
fix-gitea-branch-list

Conversation

@Meldiron
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor Author

@Meldiron Meldiron left a comment

Choose a reason for hiding this comment

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

lgtm

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 10, 2026

Greptile Summary

This PR fixes an edge case where Gitea returns an empty body (instead of an empty JSON array) for the /branches endpoint on commit-less repositories, causing listBranches to fail. It also normalises a pushed_at field across all adapters (GitLab, Gitea, Gogs) and refactors CI into a per-adapter matrix with matching Docker Compose profiles.

  • Branch list fix: listBranches in Gitea.php now calls call() with decode: false and manually JSON-decodes the body, falling back to [] on a null/empty response.
  • pushed_at parity: GitLab maps last_activity_at → pushed_at; Gitea/Gogs fall back to updated_at when pushed_at is absent.
  • CI refactor: Workflow matrix, per-adapter Docker Compose profiles, and per-adapter PHPUnit test suites are introduced so each adapter runs independently.

Confidence Score: 4/5

The source-level fixes are correct and safe to merge; one test assertion in GitHubTest uses an undefined variable and will fail as written.

The testGetRepository method in GitHubTest.php references $repo instead of $result on the two new pushed_at assertions. $repo is undefined in that scope, so the assertions will error at runtime and the new GitHub pushed_at coverage is effectively dead until corrected.

tests/VCS/Adapter/GitHubTest.php — the two new assertions in testGetRepository reference $repo (undefined) instead of $result.

Important Files Changed

Filename Overview
tests/VCS/Adapter/GitHubTest.php Adds pushed_at assertions and a skipped empty-repo branch test, but testGetRepository references undefined $repo instead of $result, breaking the new assertions.
src/VCS/Adapter/Git/Gitea.php Core fix: listBranches now decodes the response body manually to handle Gitea's empty-body edge case; also adds pushed_at normalization via updated_at fallback across create/get/list calls, and defaults rootDirectory to '*' in generateCloneCommand when empty.
src/VCS/Adapter/Git/GitLab.php Adds pushed_at key mapped from last_activity_at in both createRepository and getRepository responses for cross-adapter parity.
src/VCS/Adapter/Git/Gogs.php Mirrors Gitea's pushed_at/updated_at normalization for createRepository and listRepositories.
tests/VCS/Base.php Adds testListBranchesEmptyRepo to the base test suite and extends existing repository tests with pushed_at assertions.
phpunit.xml Splits the single test suite into per-adapter named suites to match the new matrix strategy in CI.
docker-compose.yml Assigns Docker Compose profiles to each service so CI jobs can spin up only the services needed for a given adapter.
.github/workflows/tests.yml Converts the single test job into a matrix over all five adapters, running each with its own Docker Compose profile and PHPUnit test suite.

Reviews (4): Last reviewed commit: "Merge branch 'main' into fix-gitea-branc..." | Re-trigger Greptile

@Meldiron Meldiron marked this pull request as draft April 10, 2026 14:28
Comment thread tests/VCS/Adapter/GitHubTest.php
@Meldiron Meldiron marked this pull request as ready for review April 22, 2026 08:47
Comment on lines +215 to +216
$this->assertArrayHasKey('pushed_at', $repo);
$this->assertNotFalse(\strtotime($repo['pushed_at']));
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.

P1 Wrong variable used inside testGetRepository. The result of getRepository() is stored in $result, but the two new assertions reference $repo, which is undefined in this scope. PHP will issue an undefined-variable notice and both assertions will always evaluate against null, causing them to fail.

Suggested change
$this->assertArrayHasKey('pushed_at', $repo);
$this->assertNotFalse(\strtotime($repo['pushed_at']));
$this->assertArrayHasKey('pushed_at', $result);
$this->assertNotFalse(\strtotime($result['pushed_at']));

@Meldiron
Copy link
Copy Markdown
Contributor Author

Closed in favour of #105

@Meldiron Meldiron closed this May 22, 2026
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.

1 participant