From 78507b110bb496e953f11084c8d1764dad61d0de Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 11 May 2026 16:33:30 +0530 Subject: [PATCH 1/7] Fix GitHub branch pagination --- src/VCS/Adapter/Git/GitHub.php | 25 +++++++++++---- tests/VCS/Adapter/GitHubTest.php | 54 ++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/VCS/Adapter/Git/GitHub.php b/src/VCS/Adapter/Git/GitHub.php index 56bf657d..647d8a86 100644 --- a/src/VCS/Adapter/Git/GitHub.php +++ b/src/VCS/Adapter/Git/GitHub.php @@ -738,15 +738,28 @@ public function getPullRequestFromBranch(string $owner, string $repositoryName, public function listBranches(string $owner, string $repositoryName): array { $url = "/repos/$owner/$repositoryName/branches"; + $perPage = 100; + $currentPage = 1; + $names = []; - $response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"]); + do { + $response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"], [ + 'page' => $currentPage, + 'per_page' => $perPage, + ]); - $responseBody = $response['body'] ?? []; + $responseBody = $response['body'] ?? []; - $names = []; - foreach ($responseBody as $subarray) { - $names[] = $subarray['name'] ?? ''; - } + if (!is_array($responseBody)) { + break; + } + + foreach ($responseBody as $subarray) { + $names[] = $subarray['name'] ?? ''; + } + + $currentPage++; + } while (count($responseBody) === $perPage); return $names; } diff --git a/tests/VCS/Adapter/GitHubTest.php b/tests/VCS/Adapter/GitHubTest.php index eec05722..fa1242c4 100644 --- a/tests/VCS/Adapter/GitHubTest.php +++ b/tests/VCS/Adapter/GitHubTest.php @@ -467,6 +467,60 @@ public function testGetCommit(): void $this->assertSame('7ae65094d56edafc48596ffbb77950e741e56412', $commitDetails['commitHash']); } + public function testListBranchesFetchesAllPages(): void + { + $firstPage = []; + for ($i = 1; $i <= 100; $i++) { + $firstPage[] = ['name' => "branch-{$i}"]; + } + + $secondPage = []; + for ($i = 101; $i <= 135; $i++) { + $secondPage[] = ['name' => "branch-{$i}"]; + } + + $adapter = new class (new Cache(new None()), [$firstPage, $secondPage]) extends GitHub { + /** + * @var array> + */ + public array $requests = []; + + /** + * @param array> $responses + */ + public function __construct(Cache $cache, private array $responses) + { + parent::__construct($cache); + $this->accessToken = 'test-token'; + } + + protected function call(string $method, string $path = '', array $headers = [], array $params = [], bool $decode = true) + { + $this->requests[] = [ + 'method' => $method, + 'path' => $path, + 'headers' => $headers, + 'params' => $params, + ]; + + return [ + 'headers' => ['status-code' => 200], + 'body' => array_shift($this->responses) ?? [], + ]; + } + }; + + $branches = $adapter->listBranches('appwrite', 'appwrite'); + + $this->assertCount(135, $branches); + $this->assertSame('branch-1', $branches[0]); + $this->assertSame('branch-135', $branches[134]); + $this->assertCount(2, $adapter->requests); + $this->assertSame('/repos/appwrite/appwrite/branches', $adapter->requests[0]['path']); + $this->assertSame(['page' => 1, 'per_page' => 100], $adapter->requests[0]['params']); + $this->assertSame(['page' => 2, 'per_page' => 100], $adapter->requests[1]['params']); + } + public function testGetLatestCommit(): void { $commitDetails = $this->vcsAdapter->getLatestCommit('test-kh', 'test1', 'test'); From 6f3a6065277abf6eeffbf2228af62d4c7e22f64c Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Tue, 12 May 2026 18:25:49 +0530 Subject: [PATCH 2/7] Address GitHub branch pagination review --- src/VCS/Adapter/Git/GitHub.php | 11 +++--- tests/VCS/Adapter/GitHubTest.php | 57 +++++--------------------------- 2 files changed, 14 insertions(+), 54 deletions(-) diff --git a/src/VCS/Adapter/Git/GitHub.php b/src/VCS/Adapter/Git/GitHub.php index 647d8a86..866ea95f 100644 --- a/src/VCS/Adapter/Git/GitHub.php +++ b/src/VCS/Adapter/Git/GitHub.php @@ -733,18 +733,19 @@ public function getPullRequestFromBranch(string $owner, string $repositoryName, * * @param string $owner Owner name of the repository * @param string $repositoryName Name of the GitHub repository + * @param int $perPage Number of branches to fetch per page * @return array List of branch names as array */ - public function listBranches(string $owner, string $repositoryName): array + public function listBranches(string $owner, string $repositoryName, int $perPage = 100): array { $url = "/repos/$owner/$repositoryName/branches"; - $perPage = 100; - $currentPage = 1; + $perPage = min(max($perPage, 1), 100); + $page = 1; $names = []; do { $response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"], [ - 'page' => $currentPage, + 'page' => $page, 'per_page' => $perPage, ]); @@ -758,7 +759,7 @@ public function listBranches(string $owner, string $repositoryName): array $names[] = $subarray['name'] ?? ''; } - $currentPage++; + $page++; } while (count($responseBody) === $perPage); return $names; diff --git a/tests/VCS/Adapter/GitHubTest.php b/tests/VCS/Adapter/GitHubTest.php index fa1242c4..5846cf76 100644 --- a/tests/VCS/Adapter/GitHubTest.php +++ b/tests/VCS/Adapter/GitHubTest.php @@ -467,58 +467,17 @@ public function testGetCommit(): void $this->assertSame('7ae65094d56edafc48596ffbb77950e741e56412', $commitDetails['commitHash']); } - public function testListBranchesFetchesAllPages(): void + public function testListBranchesFetchesMultiplePages(): void { - $firstPage = []; - for ($i = 1; $i <= 100; $i++) { - $firstPage[] = ['name' => "branch-{$i}"]; - } - - $secondPage = []; - for ($i = 101; $i <= 135; $i++) { - $secondPage[] = ['name' => "branch-{$i}"]; - } - - $adapter = new class (new Cache(new None()), [$firstPage, $secondPage]) extends GitHub { - /** - * @var array> - */ - public array $requests = []; - - /** - * @param array> $responses - */ - public function __construct(Cache $cache, private array $responses) - { - parent::__construct($cache); - $this->accessToken = 'test-token'; - } - - protected function call(string $method, string $path = '', array $headers = [], array $params = [], bool $decode = true) - { - $this->requests[] = [ - 'method' => $method, - 'path' => $path, - 'headers' => $headers, - 'params' => $params, - ]; - - return [ - 'headers' => ['status-code' => 200], - 'body' => array_shift($this->responses) ?? [], - ]; - } - }; + $this->assertInstanceOf(GitHub::class, $this->vcsAdapter); - $branches = $adapter->listBranches('appwrite', 'appwrite'); + /** @var GitHub $adapter */ + $adapter = $this->vcsAdapter; + $branches = $adapter->listBranches('test-kh', 'test1', 1); - $this->assertCount(135, $branches); - $this->assertSame('branch-1', $branches[0]); - $this->assertSame('branch-135', $branches[134]); - $this->assertCount(2, $adapter->requests); - $this->assertSame('/repos/appwrite/appwrite/branches', $adapter->requests[0]['path']); - $this->assertSame(['page' => 1, 'per_page' => 100], $adapter->requests[0]['params']); - $this->assertSame(['page' => 2, 'per_page' => 100], $adapter->requests[1]['params']); + $this->assertIsArray($branches); + $this->assertContains('main', $branches); + $this->assertContains('test', $branches); } public function testGetLatestCommit(): void From a413e42895083245e45fe07702cbe5032e52d22e Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Wed, 13 May 2026 19:05:06 +0530 Subject: [PATCH 3/7] Implement createBranch, add page param to listBranches, rewrite pagination test - Implement createBranch using GitHub refs API - Add page parameter to listBranches for consistency with other methods - Rewrite testListBranchesFetchesMultiplePages to use a real created repository with actual branches instead of a hardcoded external repo --- src/VCS/Adapter/Git/GitHub.php | 14 +++++++++++--- tests/VCS/Adapter/GitHubTest.php | 24 +++++++++++++++++------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/VCS/Adapter/Git/GitHub.php b/src/VCS/Adapter/Git/GitHub.php index ca7921d9..e6009750 100644 --- a/src/VCS/Adapter/Git/GitHub.php +++ b/src/VCS/Adapter/Git/GitHub.php @@ -181,7 +181,15 @@ public function createFile(string $owner, string $repositoryName, string $filepa */ public function createBranch(string $owner, string $repositoryName, string $newBranchName, string $oldBranchName): array { - throw new Exception("Not implemented"); + $latestCommit = $this->getLatestCommit($owner, $repositoryName, $oldBranchName); + $sha = $latestCommit['commitHash']; + + $response = $this->call(self::METHOD_POST, "/repos/$owner/$repositoryName/git/refs", ['Authorization' => "Bearer $this->accessToken"], [ + 'ref' => "refs/heads/$newBranchName", + 'sha' => $sha, + ]); + + return $response['body'] ?? []; } /** @@ -739,13 +747,13 @@ public function getPullRequestFromBranch(string $owner, string $repositoryName, * @param string $owner Owner name of the repository * @param string $repositoryName Name of the GitHub repository * @param int $perPage Number of branches to fetch per page + * @param int $page Page number to start fetching from * @return array List of branch names as array */ - public function listBranches(string $owner, string $repositoryName, int $perPage = 100): array + public function listBranches(string $owner, string $repositoryName, int $perPage = 100, int $page = 1): array { $url = "/repos/$owner/$repositoryName/branches"; $perPage = min(max($perPage, 1), 100); - $page = 1; $names = []; do { diff --git a/tests/VCS/Adapter/GitHubTest.php b/tests/VCS/Adapter/GitHubTest.php index 252b3f8d..f54588a4 100644 --- a/tests/VCS/Adapter/GitHubTest.php +++ b/tests/VCS/Adapter/GitHubTest.php @@ -527,15 +527,25 @@ public function testGetCommitWithInvalidHash(): void public function testListBranchesFetchesMultiplePages(): void { - $this->assertInstanceOf(GitHub::class, $this->vcsAdapter); + $repositoryName = 'test-list-branches-pages-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); - /** @var GitHub $adapter */ - $adapter = $this->vcsAdapter; - $branches = $adapter->listBranches('test-kh', 'test1', 1); + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'branch-a', static::$defaultBranch); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'branch-b', static::$defaultBranch); - $this->assertIsArray($branches); - $this->assertContains('main', $branches); - $this->assertContains('test', $branches); + /** @var GitHub $adapter */ + $adapter = $this->vcsAdapter; + $branches = $adapter->listBranches(static::$owner, $repositoryName, 1); + + $this->assertIsArray($branches); + $this->assertContains(static::$defaultBranch, $branches); + $this->assertContains('branch-a', $branches); + $this->assertContains('branch-b', $branches); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetLatestCommit(): void From 6e882521348b9f9d8b5b5794413ed8d2d8b97b34 Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Wed, 13 May 2026 19:17:36 +0530 Subject: [PATCH 4/7] Remove loop from listBranches, use page/perPage for single-page fetch --- src/VCS/Adapter/Git/GitHub.php | 28 ++++++++++------------------ tests/VCS/Adapter/GitHubTest.php | 19 +++++++++++++------ 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/VCS/Adapter/Git/GitHub.php b/src/VCS/Adapter/Git/GitHub.php index e6009750..d9b45f6c 100644 --- a/src/VCS/Adapter/Git/GitHub.php +++ b/src/VCS/Adapter/Git/GitHub.php @@ -754,28 +754,20 @@ public function listBranches(string $owner, string $repositoryName, int $perPage { $url = "/repos/$owner/$repositoryName/branches"; $perPage = min(max($perPage, 1), 100); - $names = []; - do { - $response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"], [ - 'page' => $page, - 'per_page' => $perPage, - ]); - - $responseBody = $response['body'] ?? []; - - if (!is_array($responseBody)) { - break; - } + $response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"], [ + 'page' => $page, + 'per_page' => $perPage, + ]); - foreach ($responseBody as $subarray) { - $names[] = $subarray['name'] ?? ''; - } + $statusCode = $response['headers']['status-code'] ?? 0; + $responseBody = $response['body'] ?? []; - $page++; - } while (count($responseBody) === $perPage); + if ($statusCode < 200 || $statusCode >= 300 || !is_array($responseBody)) { + return []; + } - return $names; + return array_values(array_map(fn ($branch) => $branch['name'] ?? '', $responseBody)); } /** diff --git a/tests/VCS/Adapter/GitHubTest.php b/tests/VCS/Adapter/GitHubTest.php index f54588a4..9ee4a0b1 100644 --- a/tests/VCS/Adapter/GitHubTest.php +++ b/tests/VCS/Adapter/GitHubTest.php @@ -525,7 +525,7 @@ public function testGetCommitWithInvalidHash(): void } } - public function testListBranchesFetchesMultiplePages(): void + public function testListBranchesPagination(): void { $repositoryName = 'test-list-branches-pages-' . \uniqid(); $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); @@ -537,12 +537,19 @@ public function testListBranchesFetchesMultiplePages(): void /** @var GitHub $adapter */ $adapter = $this->vcsAdapter; - $branches = $adapter->listBranches(static::$owner, $repositoryName, 1); - $this->assertIsArray($branches); - $this->assertContains(static::$defaultBranch, $branches); - $this->assertContains('branch-a', $branches); - $this->assertContains('branch-b', $branches); + $page1 = $adapter->listBranches(static::$owner, $repositoryName, 1, 1); + $this->assertCount(1, $page1); + + $page2 = $adapter->listBranches(static::$owner, $repositoryName, 1, 2); + $this->assertCount(1, $page2); + + $this->assertNotSame($page1[0], $page2[0]); + + $all = $adapter->listBranches(static::$owner, $repositoryName, 100, 1); + $this->assertContains(static::$defaultBranch, $all); + $this->assertContains('branch-a', $all); + $this->assertContains('branch-b', $all); } finally { $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); } From 9d1fde08271c5fb07f8b728efd90b8d74fda882e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 14 May 2026 09:51:02 +0200 Subject: [PATCH 5/7] Improve test coverage --- tests/VCS/Adapter/GitHubTest.php | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/VCS/Adapter/GitHubTest.php b/tests/VCS/Adapter/GitHubTest.php index 9ee4a0b1..f3a5b630 100644 --- a/tests/VCS/Adapter/GitHubTest.php +++ b/tests/VCS/Adapter/GitHubTest.php @@ -539,12 +539,10 @@ public function testListBranchesPagination(): void $adapter = $this->vcsAdapter; $page1 = $adapter->listBranches(static::$owner, $repositoryName, 1, 1); - $this->assertCount(1, $page1); + $this->assertSame(['branch-a'], $page1); $page2 = $adapter->listBranches(static::$owner, $repositoryName, 1, 2); - $this->assertCount(1, $page2); - - $this->assertNotSame($page1[0], $page2[0]); + $this->assertSame(['branch-b'], $page2); $all = $adapter->listBranches(static::$owner, $repositoryName, 100, 1); $this->assertContains(static::$defaultBranch, $all); @@ -555,6 +553,29 @@ public function testListBranchesPagination(): void } } + public function testListBranchesEmptyRepository(): void + { + $repositoryName = 'test-list-branches-empty-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $branches = $this->vcsAdapter->listBranches(static::$owner, $repositoryName); + + $this->assertIsArray($branches); + $this->assertEmpty($branches); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + } + + public function testListBranchesNonExistingRepository(): void + { + $branches = $this->vcsAdapter->listBranches(static::$owner, 'non-existing-repo-' . \uniqid()); + + $this->assertIsArray($branches); + $this->assertEmpty($branches); + } + public function testGetLatestCommit(): void { $repositoryName = 'test-get-latest-commit-' . \uniqid(); From 5e4b9360b2e5c6e05e1b91081c2ea7dc80452ae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 14 May 2026 10:05:22 +0200 Subject: [PATCH 6/7] Apply suggestion from @Meldiron --- tests/VCS/Adapter/GitHubTest.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/VCS/Adapter/GitHubTest.php b/tests/VCS/Adapter/GitHubTest.php index f3a5b630..c8085994 100644 --- a/tests/VCS/Adapter/GitHubTest.php +++ b/tests/VCS/Adapter/GitHubTest.php @@ -545,9 +545,7 @@ public function testListBranchesPagination(): void $this->assertSame(['branch-b'], $page2); $all = $adapter->listBranches(static::$owner, $repositoryName, 100, 1); - $this->assertContains(static::$defaultBranch, $all); - $this->assertContains('branch-a', $all); - $this->assertContains('branch-b', $all); + $this->assertSame([static::$defaultBranch, 'branch-a', 'branch-b'], $all); } finally { $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); } From 0fdd494e5d65ff24f6b5c8eb7a1ca60ed035e7a6 Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Thu, 14 May 2026 15:24:25 +0530 Subject: [PATCH 7/7] fix: use assertEqualsCanonicalizing for branch list order independence --- tests/VCS/Adapter/GitHubTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/VCS/Adapter/GitHubTest.php b/tests/VCS/Adapter/GitHubTest.php index c8085994..f87ff182 100644 --- a/tests/VCS/Adapter/GitHubTest.php +++ b/tests/VCS/Adapter/GitHubTest.php @@ -545,7 +545,7 @@ public function testListBranchesPagination(): void $this->assertSame(['branch-b'], $page2); $all = $adapter->listBranches(static::$owner, $repositoryName, 100, 1); - $this->assertSame([static::$defaultBranch, 'branch-a', 'branch-b'], $all); + $this->assertEqualsCanonicalizing([static::$defaultBranch, 'branch-a', 'branch-b'], $all); } finally { $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); }