Skip to content

initial unified base tests#106

Open
jaysomani wants to merge 8 commits into
utopia-php:mainfrom
jaysomani:feat/unified-base-tests
Open

initial unified base tests#106
jaysomani wants to merge 8 commits into
utopia-php:mainfrom
jaysomani:feat/unified-base-tests

Conversation

@jaysomani
Copy link
Copy Markdown
Contributor

@jaysomani jaysomani commented May 22, 2026

Migrates all shared adapter tests into Base.php following the dynamic pattern. All tests now create repos dynamically with uniqid() and clean up in finally blocks — no hardcoded owners, repos, or IDs.
Key changes:

Base.php fully rewritten with shared dynamic tests covering repository CRUD, tree, content, branches, commits, clone commands, pull requests, comments, search and owner
All adapter files implement setupAdapter() instead of setUp()
static::$owner and static::$defaultBranch used throughout
Duplicate tests removed from GiteaTest, GitLabTest, GitHubTest
Adapter-specific tests kept only where behavior genuinely differs
getOwnerName made consistent across GitLab and Gitea — requires repositoryId
Admin username changed to root across all adapters for consistency
testListBranchesEmptyRepo skipped in Base — each adapter overrides correctly

@jaysomani jaysomani changed the title inital testing initial unified base tests May 22, 2026
@jaysomani jaysomani marked this pull request as ready for review May 25, 2026 04:34
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f46cf912a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/VCS/Base.php Outdated
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 25, 2026

Greptile Summary

This PR consolidates previously duplicated adapter tests into a shared Base.php using dynamic repo names (uniqid()), finally-block cleanup, and assertEventually retry logic, while each adapter subclass now implements setupAdapter() instead of the removed createVCSAdapter() + setUp() pattern.

  • Base.php gains ~50 shared test methods covering CRUD, tree/content, branches, commits, clone, PRs, comments, search, and owner lookup; adapter-specific behaviour (webhook events, tag cloning, commit statuses) is kept in each subclass.
  • GitLab::getOwnerName drops the GET /user fallback and now requires a valid repositoryId, aligning it with the Gitea adapter's contract.
  • Default admin username in docker-compose.yml is unified to root across Gitea, Forgejo, and Gogs.

Confidence Score: 4/5

Safe to merge after adding a skip override for testGetPullRequestFromBranchNoPR in GitHubTest

GitHubTest explicitly skips testGetPullRequestFromBranch because createBranch does not work in the GitHub test environment, but testGetPullRequestFromBranchNoPR — also introduced through Base — calls the same createBranch method without a matching skip, making CI failures on the GitHub adapter suite likely

tests/VCS/Adapter/GitHubTest.php — the missing skip override for testGetPullRequestFromBranchNoPR is the one change needed before this can merge cleanly

Important Files Changed

Filename Overview
tests/VCS/Base.php Fully rewritten shared test base with dynamic repo creation, assertEventually retry logic, and finally-block cleanup; testCreatePrivateRepository does not assert the private flag
tests/VCS/Adapter/GitHubTest.php Duplicate tests removed and delegated to Base; skip overrides added for PR/comment/user tests, but testGetPullRequestFromBranchNoPR is missing a skip despite using createBranch which is flagged as non-functional in the GitHub test environment
tests/VCS/Adapter/GitLabTest.php Migrated to setupAdapter(); duplicate CRUD/clone/PR tests removed; GitLab-specific search, webhook, and commit-status tests retained
tests/VCS/Adapter/GiteaTest.php Migrated to setupAdapter(); testListBranchesEmptyRepo overridden correctly with finally-block cleanup; stale comment in the override body references a non-existent hardcoded owner
tests/VCS/Adapter/GogsTest.php Migrated to setupAdapter(); unsupported Gogs features (PR API, commit status, languages) correctly skipped; auto_init behaviour for testListBranchesEmptyRepo well-documented
tests/VCS/Adapter/ForgejoTest.php Migrated to setupAdapter(); createVCSAdapter removed; delegates entirely to GiteaTest and Base with no new issues
src/VCS/Adapter/Git/GitLab.php getOwnerName now requires a valid repositoryId (throws on null/0); the GET /user fallback path removed; change is intentional and consistent with the Gitea adapter behaviour
docker-compose.yml Default admin username for Gitea, Forgejo, and Gogs services changed from 'utopia' to 'root' for consistency across all adapters

Reviews (7): Last reviewed commit: "test: migrate additional shared tests to..." | Re-trigger Greptile

Comment thread tests/VCS/Base.php
Comment thread src/VCS/Adapter/Git/GitLab.php
Comment thread tests/VCS/Base.php Outdated
Comment thread tests/VCS/Base.php
Comment thread tests/VCS/Base.php
Comment thread tests/VCS/Base.php Outdated
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