diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da48184..63b0d28 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,9 +12,149 @@ on: branches: [main, master, '**'] pull_request: types: [opened, synchronize, reopened] + workflow_dispatch: + inputs: + provider: + description: 'Provider(s) to run (github, gitlab, gitea, or all)' + default: 'all' + keep_branch: + description: 'Keep the E2E branch/container after the run for debugging' + type: boolean + default: false + schedule: + # Weekly API-drift check: same real-provider suites, no code change required to trigger them. + - cron: '0 6 * * 1' jobs: + # `on.push.paths`/`on.pull_request.paths` would gate this *whole* workflow + # file by path -- including the release-critical `CI` job below, which must + # keep running for every push/PR regardless of path. This job instead + # computes a per-job boolean so only `provider-e2e` skips on irrelevant + # changes, while `CI`/`build-artifact` are unaffected. + changes: + name: Detect sync/provider-relevant changes + runs-on: ubuntu-latest + outputs: + e2e-relevant: ${{ steps.filter.outputs.e2e-relevant }} + steps: + - uses: actions/checkout@v6 + - uses: dorny/paths-filter@v3 + id: filter + with: + filters: | + e2e-relevant: + - 'src/services/**' + - 'src/logic/sync-manager.ts' + - 'src/utils/git-blob-sha.ts' + - 'src/utils/path.ts' + - 'src/utils/symlink.ts' + - 'e2e/**' + - 'package.json' + - 'package-lock.json' + - '.github/workflows/ci.yml' + + # Real-provider E2E: one matrix job covering GitHub, GitLab, and Gitea (see + # docs/testing/real-provider-e2e.md). + provider-e2e: + name: E2E / ${{ matrix.provider }} + needs: changes + runs-on: [self-hosted, linux, x64, 32gb-ram] + # Runs when sync/provider-relevant paths changed, or unconditionally on + # workflow_dispatch/schedule/a push to main (main always gets the full + # tier regardless of path, per the issue's CI wiring). The per-provider + # part of the gating (internal PRs/main/dispatch/schedule get every + # provider; a fork PR only gets Gitea) can't live here: job-level `if:` + # has no access to the `matrix` context (GitHub Actions error + # "Unrecognized named-value: 'matrix'" if you try) -- only step-level + # `if:` can see it. That part is done by the "Determine whether this + # provider leg should run" step below instead, gating every later step. + if: >- + needs.changes.outputs.e2e-relevant == 'true' || + github.event_name == 'workflow_dispatch' || + github.event_name == 'schedule' || + github.ref == 'refs/heads/main' + strategy: + fail-fast: false + max-parallel: 3 + matrix: + provider: [github, gitlab, gitea] + env: + E2E_GITHUB_OWNER: ${{ vars.E2E_GITHUB_OWNER }} + E2E_GITHUB_REPO: ${{ vars.E2E_GITHUB_REPO }} + E2E_GITHUB_TOKEN: ${{ secrets.E2E_GITHUB_TOKEN }} + # E2E_GITLAB_PROJECT_ID is configured as a repo *secret*, not a + # variable, on firstsun-dev/git-files-sync (confirmed via `gh secret + # list` while wiring this workflow) -- unlike E2E_GITHUB_OWNER/REPO, + # which are plain (non-sensitive) vars. + E2E_GITLAB_PROJECT_ID: ${{ secrets.E2E_GITLAB_PROJECT_ID }} + E2E_GITLAB_TOKEN: ${{ secrets.E2E_GITLAB_TOKEN }} + E2E_KEEP_BRANCH: ${{ github.event.inputs.keep_branch }} + steps: + # Per-provider gate (needs `matrix`, so it runs as a step, not the job-level + # `if:` above -- see the comment on that `if:` for why). A fork PR (head repo + # != base repo) only gets Gitea, which needs no repository secrets and can + # safely run against an untrusted fork's code; GitHub/GitLab need real sandbox + # credentials that must never be exposed to a fork PR's workflow run. All + # other events/providers run. + - name: Determine whether this provider leg should run + id: gate + run: | + run=true + if [ "${{ github.event_name }}" = "pull_request" ] \ + && [ "${{ matrix.provider }}" != "gitea" ] \ + && [ "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]; then + run=false + fi + if [ "${{ github.event_name }}" = "workflow_dispatch" ] \ + && [ "${{ github.event.inputs.provider }}" != "all" ] \ + && [ "${{ github.event.inputs.provider }}" != "${{ matrix.provider }}" ]; then + run=false + fi + echo "run=$run" >> "$GITHUB_OUTPUT" + + - uses: actions/checkout@v6 + if: steps.gate.outputs.run == 'true' + + - uses: actions/setup-node@v6 + if: steps.gate.outputs.run == 'true' + with: + node-version: '22' + cache: npm + + - run: npm ci + if: steps.gate.outputs.run == 'true' + + - name: Run provider E2E + if: steps.gate.outputs.run == 'true' + run: node scripts/run-e2e-ci.mjs --provider=${{ matrix.provider }} + + # Aggregates the matrix into a single required status so branch protection + # only has to reference one check name (see docs/testing/real-provider-e2e.md + # for the "Gitea required, GitHub/GitLab not required at branch-protection + # level" split -- required-vs-optional per *provider* still comes from the + # "Determine whether this provider leg should run" step above; this gate + # only asks "did whatever ran, pass?"). A gated-off leg's steps are all + # skipped without failing the job, so it still reports "success" here. + # `if: always()` so a real provider-e2e failure/cancellation is caught + # here and blocks CI/release, instead of GitHub Actions silently treating + # an upstream failure as "this job never needed to run". + e2e-gate: + name: E2E gate + needs: provider-e2e + if: always() + runs-on: ubuntu-latest + steps: + - name: Check provider-e2e result + run: | + result="${{ needs.provider-e2e.result }}" + echo "provider-e2e result: $result" + if [ "$result" != "success" ] && [ "$result" != "skipped" ]; then + echo "::error::provider-e2e failed or was cancelled ($result) -- blocking CI/release." + exit 1 + fi + CI: + needs: e2e-gate uses: firstsun-dev/.github/.github/workflows/obsidian-plugin-ci.yml@v1 with: plugin-id: "git-file-sync" diff --git a/docs/testing/real-provider-e2e.md b/docs/testing/real-provider-e2e.md new file mode 100644 index 0000000..1ad77d9 --- /dev/null +++ b/docs/testing/real-provider-e2e.md @@ -0,0 +1,123 @@ +# Real-provider E2E + +Issue #57. Real `SyncManager`/`GitHubService`/`GitLabService`/`GiteaService` code run +against real GitHub, GitLab, and Gitea servers, with every remote assertion made through an +independent verifier (raw REST calls, never the service under test reading back its own +write). See `e2e/` for the harness itself: + +- `e2e/providers/` — one adapter per provider (`provision()` -> real, already-configured + `GitServiceInterface`; `teardown()`). +- `e2e/provision/` — GitHub/GitLab: validates credentials against a dedicated sandbox + repo/project and creates a run-specific branch. Gitea: provisions a pinned Docker container + from scratch. +- `e2e/verifier/` — one `RemoteVerifier` per provider, raw API calls only. +- `e2e/suites/{github,gitlab,gitea}.e2e.test.ts` — provider contract suites (create/read/ + update/delete/batch/rename, plus provider-specific regressions). +- `e2e/suites/sync-manager.e2e.test.ts` — one suite, parametrized by `E2E_PROVIDER`, covering + `SyncManager` itself (push/pull/conflict/rename/delete/batch) against a real provider with an + in-memory fake Vault (`e2e/shim/fake-vault.ts`) standing in for the Obsidian filesystem + boundary — see that file's header comment for why the vault is the only thing faked. + +## Running locally + +```sh +npm run test:e2e -- --provider gitea # no credentials needed, runs a real Gitea in Docker +npm run test:e2e -- --provider github # needs E2E_GITHUB_* below +npm run test:e2e -- --provider gitlab # needs E2E_GITLAB_* below +``` + +Each command runs that provider's contract suite *and* the SyncManager suite in one process +(`scripts/run-e2e.mjs`). Export credentials in your shell before running (there is no +`.env`-style file loader in this harness — plain `process.env`, matching `e2e/config/env.ts`): + +| Var | Required for | Notes | +|---|---|---| +| `E2E_GITHUB_OWNER` | github | e.g. `firstsun-dev` | +| `E2E_GITHUB_REPO` | github | dedicated sandbox repo — **never** a real user's repo | +| `E2E_GITHUB_TOKEN` | github | fine-grained PAT, scoped to that one repo, Contents: Read and write | +| `E2E_GITHUB_BASE_BRANCH` | github (optional) | defaults to `main` | +| `E2E_GITLAB_PROJECT_ID` | gitlab | dedicated sandbox project | +| `E2E_GITLAB_TOKEN` | gitlab | token with `api` scope on that project — `write_repository` alone is not enough, the verifier and branch setup use REST endpoints outside its coverage | +| `E2E_GITLAB_BASE_URL` | gitlab (optional) | defaults to `https://gitlab.com` | +| `E2E_KEEP_BRANCH` | any (optional) | `1`/`true` skips teardown (branch for GitHub/GitLab, container for Gitea) so you can inspect a failing run | + +Gitea needs Docker locally and nothing else — see `e2e/provision/gitea-provision.ts`. + +## CI + +`.github/workflows/ci.yml` runs a `provider-e2e` matrix job (`github`, `gitlab`, `gitea`) via +`scripts/run-e2e-ci.mjs`, gated on relevant paths (`src/services/**`, +`src/logic/sync-manager.ts`, `e2e/**`, etc. — computed by the `changes` job, since GitHub +Actions' own `on.*.paths` would gate the *entire* workflow file, including the always-must-run +`CI`/release job). It always runs in full on `workflow_dispatch`, `schedule` (weekly, Monday +06:00 UTC, for API-drift detection), and pushes to `main`. + +**Secrets/variables** (repo-level, `firstsun-dev/git-files-sync`; confirmed already configured +via `gh secret list` / `gh variable list` while wiring this workflow): + +| Name | Kind | +|---|---| +| `E2E_GITHUB_TOKEN` | secret | +| `E2E_GITHUB_OWNER` | variable | +| `E2E_GITHUB_REPO` | variable | +| `E2E_GITLAB_PROJECT_ID` | secret (not a variable — it's treated as sensitive here) | +| `E2E_GITLAB_TOKEN` | secret | + +**Fork PRs** only run the Gitea cell (checked in the `Determine whether this provider leg should +run` step — GitHub Actions job-level `if:` can't reference the `matrix` context, so this can't +live on the job itself; it gates every later step instead) — GitHub/GitLab need +real credentials that must never be exposed to an untrusted fork's workflow run. Gitea needs no +repo secrets at all, so it's safe to run unconditionally. + +**Missing credentials are always a hard failure**, never a silent skip, for any cell that +actually runs (`scripts/run-e2e-ci.mjs` checks required env vars up front) — the job-level `if:` +above is what decides whether a cell *should* run for a given event; once it runs, it's expected +to have what it needs. + +## Release gating + +``` +changes -> provider-e2e [github | gitlab | gitea, parallel] -> e2e-gate -> CI (shared workflow, includes semantic-release) +``` + +`e2e-gate` runs with `if: always()` and treats `provider-e2e`'s aggregate result as pass-through +on `success` or `skipped` (the latter covers path-filtered-out runs), and a hard failure on +anything else — so a real provider regression blocks the release instead of shipping and being +caught after the fact. + +**Branch protection** (not something this repo checkout can change — a GitHub repo-settings +change, left for whoever has admin access): add `E2E / gitea` as a required status check. +GitHub/GitLab (`E2E / github`, `E2E / gitlab`) are deliberately **not** required at the +branch-protection level, so a fork PR (which only runs Gitea) is never wedged by checks it +structurally cannot produce — internal-PR/main-branch release gating still depends on them +through the `e2e-gate`/`CI` job dependency chain above, just not through branch protection. + +## Cleanup / troubleshooting + +- **Stale `gfs-e2e--*` branch** (GitHub/GitLab only — Gitea's whole container is + destroyed in `afterAll`): `scripts/run-e2e-ci.mjs` runs `scripts/e2e-sweep-branches.mjs` + before every CI run, which best-effort deletes any branch of that pattern older than 24h. Run + it manually (`node scripts/e2e-sweep-branches.mjs --provider github`) if you need it sooner. +- **Inspecting a failing run**: set `E2E_KEEP_BRANCH=1` before running so teardown is skipped, + then look at the branch/container directly. Remember to clean it up yourself afterward, or let + the sweeper (GitHub/GitLab) catch it after 24h. +- **Gitea container port/name clashes**: every Docker resource is namespaced per run + (`e2e/namespace.ts`, `gfs-e2e-gitea--` in CI, `gfs-e2e-gitea-local-` + locally), so concurrent runs on the same Docker host don't collide — a leftover container from + an interrupted local run can just be removed manually (`docker rm -f `). +- **`E2E_PROVIDER is not set` error**: the E2E vitest config (`vitest.e2e.config.ts`) refuses to + run directly under `npx vitest` — always go through `npm run test:e2e -- --provider ` (or + `scripts/run-e2e-ci.mjs` in CI), which sets it. + +## Known gaps + +- SyncManager E2E against GitHub/GitLab is written to the same harness as Gitea (no + provider-specific code) but has only been run end-to-end locally against Gitea (Docker, + no external credentials available in that environment) — not yet actually executed against + live GitHub/GitLab sandboxes. Lint/build/typecheck pass for all three. +- The `provider-e2e` matrix job targets `runs-on: [self-hosted, linux, x64, 32gb-ram]` per the + issue's runner-fleet revision; its actual execution on that fleet, and the `e2e-gate` -> + `CI` dependency chain end-to-end in a real workflow run, are unverified from this checkout + (no self-hosted runner access here). +- Branch-protection required-check configuration (`E2E / gitea`) is a manual follow-up for + whoever has admin access to the repo. diff --git a/e2e/provision/docker.ts b/e2e/provision/docker.ts index e860a84..5ee0d9a 100644 --- a/e2e/provision/docker.ts +++ b/e2e/provision/docker.ts @@ -31,6 +31,20 @@ export async function removeContainer(name: string): Promise { await dockerAllowFailure(['rm', '-f', name]); } +/** Best-effort: the container's own stdout/stderr, for diagnosing a readiness + * timeout (e.g. a slow/failed startup) directly from CI output instead of + * needing shell access to the runner. Never throws. `docker logs` writes the + * container's stdout/stderr to its own stdout/stderr respectively, so both + * are captured and combined, not just stdout. */ +export async function containerLogsAllowFailure(name: string, tailLines = 200): Promise { + try { + const { stdout, stderr } = await execFileAsync('docker', ['logs', '--tail', String(tailLines), name]); + return [stdout, stderr].filter(Boolean).join('\n').trim(); + } catch (e) { + return `(failed to fetch container logs: ${e instanceof Error ? e.message : String(e)})`; + } +} + /** Reads back the dynamic host port Docker assigned for a `-p 0:` mapping. */ export async function hostPortFor(containerName: string, containerPort: number): Promise { const output = await docker(['port', containerName, String(containerPort)]); diff --git a/e2e/provision/gitea-provision.ts b/e2e/provision/gitea-provision.ts index fe768f1..0487ad9 100644 --- a/e2e/provision/gitea-provision.ts +++ b/e2e/provision/gitea-provision.ts @@ -4,7 +4,7 @@ import { promisify } from 'node:util'; import { runNamespace } from '../namespace'; import { globalSecrets, logInfo } from '../redact'; import { giteaImage, timeouts } from '../config/env'; -import { createNetwork, removeNetwork, removeContainer, hostPortFor, waitUntilReady, docker } from './docker'; +import { createNetwork, removeNetwork, removeContainer, hostPortFor, waitUntilReady, docker, containerLogsAllowFailure } from './docker'; const execFileAsync = promisify(execFile); @@ -86,9 +86,15 @@ export async function provisionGitea(): Promise { return { baseUrl, owner: ADMIN_USERNAME, repo: SANDBOX_REPO, token, containerName, networkName }; } catch (e) { - // Provisioning failed partway through — clean up what we started before rethrowing. + // Provisioning failed partway through. A readiness timeout in particular + // gives no clue *why* Gitea never came up (self-hosted runner Docker/ + // network hiccup vs. a real startup failure) without runner shell access + // -- attach the container's own logs to the error before it's torn down, + // so a future CI failure is diagnosable straight from the job output. + const logs = await containerLogsAllowFailure(containerName); await teardownGitea({ containerName, networkName } as GiteaEnvironment); - throw e; + const message = e instanceof Error ? e.message : String(e); + throw new Error(`${message}\n\n-- gitea container logs (tail) --\n${logs}`); } } @@ -128,6 +134,10 @@ async function createSandboxRepo(baseUrl: string, token: string): Promise /** Best-effort cleanup — safe to call even if provisioning only partially completed. */ export async function teardownGitea(env: Pick): Promise { + if (process.env.E2E_KEEP_BRANCH === '1' || process.env.E2E_KEEP_BRANCH === 'true') { + logInfo(`E2E_KEEP_BRANCH set — leaving container ${env.containerName} running for debugging`); + return; + } logInfo(`Removing container ${env.containerName}`); await removeContainer(env.containerName); logInfo(`Removing network ${env.networkName}`); diff --git a/e2e/provision/github-provision.ts b/e2e/provision/github-provision.ts index 956d9b6..e5f0bdd 100644 --- a/e2e/provision/github-provision.ts +++ b/e2e/provision/github-provision.ts @@ -82,6 +82,10 @@ export async function provisionGitHub(): Promise { /** Best-effort cleanup — safe to call even if provisioning only partially completed. */ export async function teardownGitHub(env: GitHubEnvironment): Promise { + if (process.env.E2E_KEEP_BRANCH === '1' || process.env.E2E_KEEP_BRANCH === 'true') { + logInfo(`E2E_KEEP_BRANCH set — leaving run branch ${env.branch} in place for debugging`); + return; + } logInfo(`Removing run branch ${env.branch}`); try { await fetch(`${API_BASE}/repos/${env.owner}/${env.repo}/git/refs/heads/${encodeURIComponent(env.branch)}`, { diff --git a/e2e/provision/gitlab-provision.ts b/e2e/provision/gitlab-provision.ts index bd434d3..d3d52da 100644 --- a/e2e/provision/gitlab-provision.ts +++ b/e2e/provision/gitlab-provision.ts @@ -51,6 +51,10 @@ export async function provisionGitLab(): Promise { /** Best-effort cleanup — deletes the run-specific branch. Must not throw. */ export async function teardownGitLab(env: GitLabEnvironment): Promise { + if (process.env.E2E_KEEP_BRANCH === '1' || process.env.E2E_KEEP_BRANCH === 'true') { + logInfo(`E2E_KEEP_BRANCH set — leaving branch ${env.branch} in place for debugging`); + return; + } try { logInfo(`Removing branch ${env.branch}`); const encodedProjectId = encodeURIComponent(env.projectId); diff --git a/e2e/shim/fake-vault.ts b/e2e/shim/fake-vault.ts new file mode 100644 index 0000000..d103f50 --- /dev/null +++ b/e2e/shim/fake-vault.ts @@ -0,0 +1,73 @@ +import type { App } from 'obsidian'; +import { TFile } from './obsidian-request-url'; + +/** + * Real in-memory Obsidian Vault/App stand-in for SyncManager E2E (see + * e2e/suites/sync-manager.e2e.test.ts) — not a `vi.fn()` mock. The point of + * SyncManager E2E is to exercise real `SyncManager` + real provider service + * code against a real Git server; the *only* thing worth faking is the + * Obsidian filesystem boundary, so this implements exactly the `vault`/ + * `vault.adapter` surface `src/logic/sync-manager.ts` actually touches + * (confirmed by reading it) as a plain `Map`. + */ +export class FakeVault { + private readonly files = new Map(); + + /** Seeds local vault state directly, bypassing any sync logic. */ + writeLocal(path: string, content: string | ArrayBuffer): void { + this.files.set(path, content); + } + + has(path: string): boolean { + return this.files.has(path); + } + + /** Mirrors what Obsidian does on a vault rename: same content, new path. */ + renameLocal(oldPath: string, newPath: string): void { + const content = this.files.get(oldPath); + if (content === undefined) throw new Error(`fake vault: no local file at ${oldPath}`); + this.files.delete(oldPath); + this.files.set(newPath, content); + } + + readonly adapter = { + exists: async (path: string): Promise => this.files.has(path), + read: async (path: string): Promise => { + const content = this.files.get(path); + if (typeof content !== 'string') throw new Error(`fake vault: no text file at ${path}`); + return content; + }, + readBinary: async (path: string): Promise => { + const content = this.files.get(path); + if (!(content instanceof ArrayBuffer)) throw new Error(`fake vault: no binary file at ${path}`); + return content; + }, + write: async (path: string, content: string): Promise => { + this.files.set(path, content); + }, + writeBinary: async (path: string, content: ArrayBuffer): Promise => { + this.files.set(path, content); + }, + // ensureParentDirs (src/utils/vault-path.ts) tolerates mkdir failures; + // there are no real directories to create in an in-memory map. + mkdir: async (): Promise => {}, + }; + + readonly vault = { + read: async (file: TFile): Promise => this.adapter.read(file.path), + readBinary: async (file: TFile): Promise => this.adapter.readBinary(file.path), + modify: async (file: TFile, content: string): Promise => { + this.files.set(file.path, content); + }, + modifyBinary: async (file: TFile, content: ArrayBuffer): Promise => { + this.files.set(file.path, content); + }, + getFileByPath: (path: string): TFile | null => (this.files.has(path) ? new TFile(path) : null), + adapter: this.adapter, + }; +} + +/** Casts a FakeVault into the shape SyncManager expects as its `App` — it only ever touches `app.vault.*`. */ +export function fakeApp(fakeVault: FakeVault): App { + return { vault: fakeVault.vault } as unknown as App; +} diff --git a/e2e/shim/obsidian-request-url.ts b/e2e/shim/obsidian-request-url.ts index 8ac3e0c..1897907 100644 --- a/e2e/shim/obsidian-request-url.ts +++ b/e2e/shim/obsidian-request-url.ts @@ -58,4 +58,85 @@ export async function requestUrl(request: RequestUrlParam | string): Promise `src/settings-implementation.ts` (which bundles those + * pure functions in the same file as the `GitLabSyncSettingTab` UI class) -> + * `src/ui/FolderSuggest.ts`, pulling in `PluginSettingTab`, `TextComponent`, + * `AbstractInputSuggest`, and `TFolder` as real top-level `class X extends Y` + * values too, even though the SyncManager E2E suite never triggers the + * settings UI itself. Splitting those pure functions out of + * settings-implementation.ts to avoid this is a bigger production-code + * change than this E2E harness should make; stubbing the shape here is the + * narrower fix. */ +export class Modal { + app: unknown; + constructor(app?: unknown) { this.app = app; } + open(): void {} + close(): void {} +} + +export class PluginSettingTab { + constructor(_app?: unknown, _plugin?: unknown) {} +} +export class TextComponent {} +export class AbstractInputSuggest<_T> { + constructor(_app: unknown, _inputEl: unknown) {} +} +export class TFolder { + path: string; + constructor(path: string) { this.path = path; } +} +export class Setting { + constructor(_containerEl?: unknown) {} +} + +export class TFile { + path: string; + name: string; + constructor(path: string) { + this.path = path; + this.name = path.split('/').pop() ?? path; + } +} + +export class Notice { + constructor(_message?: string, _timeout?: number) {} + setMessage(): this { return this; } + hide(): void {} +} + +export const Platform = { isDesktopApp: false, isMobile: false }; + +export class FileSystemAdapter { + getBasePath(): string { return '/e2e/fake-vault'; } +} diff --git a/e2e/suites/gitea.e2e.test.ts b/e2e/suites/gitea.e2e.test.ts index 5f15117..e453013 100644 --- a/e2e/suites/gitea.e2e.test.ts +++ b/e2e/suites/gitea.e2e.test.ts @@ -18,7 +18,9 @@ describe('GiteaService E2E', () => { }, timeouts.containerReadyMs + 30_000); afterAll(async () => { - await adapter.teardown(ctx); + // Guard against beforeAll failing before ctx is assigned (e.g. Docker/ + // container-readiness failure) — teardown must not throw in that case either. + if (ctx) await adapter.teardown(ctx); }); it('testConnection reports the repo and branch as reachable', async () => { diff --git a/e2e/suites/sync-manager.e2e.test.ts b/e2e/suites/sync-manager.e2e.test.ts new file mode 100644 index 0000000..6fa9893 --- /dev/null +++ b/e2e/suites/sync-manager.e2e.test.ts @@ -0,0 +1,271 @@ +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { randomBytes } from 'node:crypto'; +import { SyncManager } from '../../src/logic/sync-manager'; +import { SyncPlanModal, SyncPlanDirection } from '../../src/ui/SyncPlanModal'; +import { SyncConflictModal } from '../../src/ui/SyncConflictModal'; +// `import type` deliberately, not a value import: src/settings.ts also +// exports settings-tab UI (GitLabSyncSettingTab -> FolderSuggest -> +// AbstractInputSuggest etc.) which pulls in far more of `obsidian` than this +// suite's minimal shim provides. A type-only import is erased entirely, so +// none of that module ever loads. +import type { GitLabFilesPushSettings } from '../../src/settings'; +import type { TFile as ObsidianTFile } from 'obsidian'; +import { FakeVault, fakeApp } from '../shim/fake-vault'; +import { TFile } from '../shim/obsidian-request-url'; +import { currentProvider, timeouts } from '../config/env'; +import { GiteaE2EAdapter } from '../providers/gitea-adapter'; +import { GitHubE2EAdapter } from '../providers/github-adapter'; +import { GitLabE2EAdapter } from '../providers/gitlab-adapter'; +import type { ProviderE2EAdapter, ProvisionedProvider } from '../providers/provider-adapter'; +import type { RemoteVerifier } from '../verifier/verifier-contract'; + +// Every push/pull SyncManager does shows a plan-review modal first; bare +// vi.mock (automock) + a per-suite auto-confirm implementation is the same +// pattern tests/logic/sync-manager.test.ts uses for unit tests. Conflict +// modal is left as the bare automock default (does nothing, never invokes +// onChoose) -- that's the real production behavior too: pushFile/pullFile +// return before the conflict modal resolves, so a bare mock is already +// correct, not a simplification of what's being tested. +vi.mock('../../src/ui/SyncPlanModal'); +vi.mock('../../src/ui/SyncConflictModal'); + +interface AdapterWithVerifier extends ProvisionedProvider { + verifier: RemoteVerifier; +} + +function adapterFor(provider: string): ProviderE2EAdapter { + if (provider === 'github') return new GitHubE2EAdapter(); + if (provider === 'gitlab') return new GitLabE2EAdapter(); + return new GiteaE2EAdapter(); +} + +/** + * The E2E `obsidian` shim's `TFile` (e2e/shim/obsidian-request-url.ts) is a + * separate, minimal class from the real `obsidian` package's `TFile` type + * that `SyncManager`'s public methods are typed against -- vitest's runtime + * module alias makes them the same *value* when this suite actually runs, + * but `tsc` type-checks against the real `obsidian` .d.ts regardless of that + * runtime alias, so passing the shim class straight into e.g. `pushFile` + * needs this cast to satisfy the type checker. + */ +function asTFile(path: string): ObsidianTFile { + return new TFile(path) as unknown as ObsidianTFile; +} + +function makeSettings(branch: string): GitLabFilesPushSettings { + return { + serviceType: 'gitea', + gitlabToken: '', gitlabBaseUrl: '', projectId: '', + githubToken: '', githubOwner: '', githubRepo: '', + giteaToken: '', giteaBaseUrl: '', giteaOwner: '', giteaRepo: '', + branch, + syncMetadata: {}, + rootPath: '', + vaultFolder: '', + symlinkHandling: 'skip', + ignorePatterns: '', + lastSeenVersion: '', + bannerDismissedVersion: '', + language: 'system', + autoRefreshOnStartup: true, + }; +} + +/** + * Real SyncManager + real production provider service (see e2e/providers/), + * driven against whichever provider `E2E_PROVIDER` selects -- the same + * adapter/verifier/provisioner the contract suites use, so this suite adds + * no provider-specific logic of its own (see e2e/verifier/verifier-contract.ts + * for what "independent verification" means here). Only the Obsidian + * filesystem boundary is faked (e2e/shim/fake-vault.ts); everything else is + * the real code path. + */ +describe('SyncManager E2E', () => { + const provider = currentProvider(); + const adapter = adapterFor(provider); + let ctx: AdapterWithVerifier; + const runId = randomBytes(4).toString('hex'); + const path = (name: string) => `e2e-sync-${runId}/${name}`; + + beforeAll(async () => { + ctx = (await adapter.provision()) as AdapterWithVerifier; + }, timeouts.containerReadyMs + 30_000); + + afterAll(async () => { + // Guard against beforeAll failing before ctx is assigned (e.g. Docker/ + // container-readiness failure, missing credentials) — teardown must not + // throw in that case either. + if (ctx) await adapter.teardown(ctx); + }); + + function newManager(vault: FakeVault, settings: GitLabFilesPushSettings): SyncManager { + vi.mocked(SyncPlanModal).mockImplementation(function ( + this: SyncPlanModal, _app: unknown, _plan: unknown, _direction: SyncPlanDirection, onConfirm: () => void + ) { + onConfirm(); + return this; + } as never); + return new SyncManager(fakeApp(vault), ctx.service, settings, undefined, () => false); + } + + it('pushes a new local file, verified independently of the service', async () => { + const filePath = path('new-file.md'); + const vault = new FakeVault(); + vault.writeLocal(filePath, '# local content'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + + const result = await manager.pushFile(filePath); + + expect(result?.sha).toBeTruthy(); + const remote = await ctx.verifier.getFile(filePath, ctx.branch); + expect(remote?.content).toBe('# local content'); + expect(remote?.sha).toBe(result?.sha); + expect(settings.syncMetadata[filePath]?.lastSyncedSha).toBe(remote?.sha); + }); + + it('does not create a remote mutation when pushing an unchanged file', async () => { + const filePath = path('unchanged.md'); + const vault = new FakeVault(); + vault.writeLocal(filePath, 'steady state'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + await manager.pushFile(filePath); + + const shasBefore = await ctx.verifier.listCommitShas(ctx.branch); + const result = await manager.pushFile(filePath); + const shasAfter = await ctx.verifier.listCommitShas(ctx.branch); + + expect(result?.sha).toBeTruthy(); + expect(shasAfter[0]).toBe(shasBefore[0]); + }); + + it('pulls a remote update into the local vault', async () => { + const filePath = path('to-pull.md'); + // Seed the remote directly (not via SyncManager/pullFile), so this + // vault's SyncManager has no syncMetadata baseline for the path yet -- + // e.g. the file was already in the vault before sync was ever run for + // it. That's what makes this a plain pull rather than a conflict: see + // sync-manager.ts's pull conflict check, which only fires when a prior + // lastSyncedSha exists and no longer matches the remote (exercised by + // the "conflict protection" test below, which does establish a + // baseline first). + await ctx.service.pushFile(filePath, 'v1', ctx.branch, 'e2e: seed remote file'); + const vault = new FakeVault(); + vault.writeLocal(filePath, 'v1'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + + // Remote changes out from under the vault -- via the real production + // service, same as another client pushing, not via SyncManager. + const remoteBefore = await ctx.verifier.getFile(filePath, ctx.branch); + await ctx.service.pushFile(filePath, 'v2 from another client', ctx.branch, 'e2e: simulate remote update', remoteBefore?.sha); + + await manager.pullFile(filePath); + + expect(await vault.adapter.read(filePath)).toBe('v2 from another client'); + const remoteAfter = await ctx.verifier.getFile(filePath, ctx.branch); + expect(settings.syncMetadata[filePath]?.lastSyncedSha).toBe(remoteAfter?.sha); + }); + + it('does not overwrite the remote or falsely mark synced when both sides changed', async () => { + const filePath = path('conflict.md'); + const vault = new FakeVault(); + vault.writeLocal(filePath, 'baseline'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + await manager.pushFile(filePath); + const baselineMeta = settings.syncMetadata[filePath]; + + // Diverge both sides from the synced baseline. + vault.writeLocal(filePath, 'local edit'); + const remoteBaseline = await ctx.verifier.getFile(filePath, ctx.branch); + await ctx.service.pushFile(filePath, 'remote edit', ctx.branch, 'e2e: diverge remote', remoteBaseline?.sha); + + const conflictCallsBefore = vi.mocked(SyncConflictModal).mock.calls.length; + const result = await manager.pushFile(filePath); + + expect(result).toBeUndefined(); + expect(vi.mocked(SyncConflictModal).mock.calls.length).toBe(conflictCallsBefore + 1); + const remoteAfter = await ctx.verifier.getFile(filePath, ctx.branch); + expect(remoteAfter?.content).toBe('remote edit'); + expect(settings.syncMetadata[filePath]).toEqual(baselineMeta); + }); + + it('renames/moves a file in exactly one commit, verified independently of the service', async () => { + const oldPath = path('rename/old.md'); + const newPath = path('rename/new.md'); + const vault = new FakeVault(); + vault.writeLocal(oldPath, 'move me'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + await manager.pushFile(oldPath); + + vault.renameLocal(oldPath, newPath); + await manager.trackRename(newPath, oldPath); + // Just the current HEAD, not a full list: the sandbox repo's base branch + // already carries pre-existing history (e.g. 47 commits on this GitHub + // sandbox's `main` at the time of writing), so asserting on + // listCommitShas(...).length would silently start failing forever once + // that history exceeds the API's default page size (30) -- the "before" + // and "after" calls both cap at the same page size and stop reflecting + // real growth. Comparing HEAD-before against the two newest commits + // after is exact regardless of total history depth. + const [headBefore] = await ctx.verifier.listCommitShas(ctx.branch, 1); + + // Rename detection only runs off a real TFile (sync-manager.ts checks + // `!isString && fileOrPath instanceof TFile` before consulting + // `renamedFrom`) -- a plain path string, as every other scenario in + // this suite uses, always takes the plain-push branch instead, same + // as it does in production when the caller doesn't have a TFile handy. + await manager.pushFile(asTFile(newPath)); + + expect(await ctx.verifier.fileMissing(oldPath, ctx.branch)).toBe(true); + const remote = await ctx.verifier.getFile(newPath, ctx.branch); + expect(remote?.content).toBe('move me'); + const [headAfter, headAfterParent] = await ctx.verifier.listCommitShas(ctx.branch, 2); + expect(headAfter).not.toBe(headBefore); + expect(headAfterParent).toBe(headBefore); + }); + + it('deletes a file via the real service, verified independently', async () => { + // Deletion isn't a SyncManager method -- src/ui/SyncStatusView.ts calls + // gitService.deleteFile directly, so this reproduces that real path. + const filePath = path('to-delete.md'); + const vault = new FakeVault(); + vault.writeLocal(filePath, 'delete me'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + await manager.pushFile(filePath); + expect(await ctx.verifier.fileMissing(filePath, ctx.branch)).toBe(false); + + await ctx.service.deleteFile(filePath, ctx.branch, 'e2e: delete file'); + await manager.clearMetadata(filePath); + + expect(await ctx.verifier.fileMissing(filePath, ctx.branch)).toBe(true); + expect(settings.syncMetadata[filePath]).toBeUndefined(); + }); + + it('pushes a batch of local files in exactly one commit, verified independently', async () => { + const paths = [path('batch/a.md'), path('batch/b.md'), path('batch/c.md')]; + const vault = new FakeVault(); + for (const p of paths) vault.writeLocal(p, `content for ${p}`); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + // See the rename test above for why this compares HEAD-before against + // the two newest commits after, rather than list length. + const [headBefore] = await ctx.verifier.listCommitShas(ctx.branch, 1); + + const results = await manager.pushAllFiles(paths); + + expect(results.success).toBe(paths.length); + expect(results.failed).toBe(0); + for (const p of paths) { + const remote = await ctx.verifier.getFile(p, ctx.branch); + expect(remote?.content).toBe(`content for ${p}`); + } + const [headAfter, headAfterParent] = await ctx.verifier.listCommitShas(ctx.branch, 2); + expect(headAfter).not.toBe(headBefore); + expect(headAfterParent).toBe(headBefore); + }); +}); diff --git a/e2e/verifier/gitea-verifier.ts b/e2e/verifier/gitea-verifier.ts index 910d55c..39867a1 100644 --- a/e2e/verifier/gitea-verifier.ts +++ b/e2e/verifier/gitea-verifier.ts @@ -45,4 +45,12 @@ export class GiteaVerifier implements RemoteVerifier { async fileMissing(path: string, ref: string): Promise { return (await this.getFile(path, ref)) === null; } + + async listCommitShas(ref: string, perPage = 30): Promise { + const url = `${this.baseUrl}/api/v1/repos/${this.owner}/${this.repo}/commits?sha=${encodeURIComponent(ref)}&limit=${perPage}`; + const res = await fetch(url, { headers: this.headers() }); + if (!res.ok) throw new Error(`GiteaVerifier.listCommitShas failed: ${res.status} ${await res.text()}`); + const data = await res.json() as Array<{ sha: string }>; + return data.map(item => item.sha); + } } diff --git a/e2e/verifier/github-verifier.ts b/e2e/verifier/github-verifier.ts index ffbbab3..3fc8353 100644 --- a/e2e/verifier/github-verifier.ts +++ b/e2e/verifier/github-verifier.ts @@ -56,7 +56,6 @@ export class GitHubVerifier implements RemoteVerifier { return tree.find(item => item.path === path)?.mode ?? null; } - /** GitHub-specific: commit shas on `ref`, newest first — used to assert a batch/rename landed as exactly one commit. */ async listCommitShas(ref: string, perPage = 30): Promise { const url = `${API_BASE}/repos/${this.owner}/${this.repo}/commits?sha=${encodeURIComponent(ref)}&per_page=${perPage}`; const res = await fetch(url, { headers: this.headers() }); diff --git a/e2e/verifier/gitlab-verifier.ts b/e2e/verifier/gitlab-verifier.ts index f2f17dd..e2c5c37 100644 --- a/e2e/verifier/gitlab-verifier.ts +++ b/e2e/verifier/gitlab-verifier.ts @@ -53,6 +53,14 @@ export class GitLabVerifier implements RemoteVerifier { return (await this.getFile(path, ref)) === null; } + async listCommitShas(ref: string, perPage = 30): Promise { + const url = `${this.baseUrl}/api/v4/projects/${this.encodedProjectId}/repository/commits?ref_name=${encodeURIComponent(ref)}&per_page=${perPage}`; + const res = await fetch(url, { headers: this.headers() }); + if (!res.ok) throw new Error(`GitLabVerifier.listCommitShas failed: ${res.status} ${await res.text()}`); + const data = await res.json() as Array<{ id: string }>; + return data.map(item => item.id); + } + /** * Fetches the file's `last_commit_id` (GitLab's optimistic-locking * revision) directly, independent of GitLabService.getFile. Used by the diff --git a/e2e/verifier/verifier-contract.ts b/e2e/verifier/verifier-contract.ts index 0f6eddb..6ec569c 100644 --- a/e2e/verifier/verifier-contract.ts +++ b/e2e/verifier/verifier-contract.ts @@ -15,4 +15,7 @@ export interface RemoteVerifier { /** True if `path` does not exist at `ref` (used to verify deletes/renames-away). */ fileMissing(path: string, ref: string): Promise; + + /** Commit shas on `ref`, newest first — used to assert a batch/rename/push landed as exactly N new commits, without trusting the service under test's own commit count. */ + listCommitShas(ref: string, perPage?: number): Promise; } diff --git a/scripts/e2e-sweep-branches.mjs b/scripts/e2e-sweep-branches.mjs new file mode 100644 index 0000000..18fd769 --- /dev/null +++ b/scripts/e2e-sweep-branches.mjs @@ -0,0 +1,112 @@ +#!/usr/bin/env node +/* + * Best-effort cleanup for leftover `gfs-e2e--*` branches (see + * e2e/namespace.ts) left behind by a crashed/cancelled CI run -- a normal + * run deletes its own branch in teardown (e2e/provision/{github,gitlab}- + * provision.ts). Gitea needs no sweeper: its whole container, not just a + * branch, is torn down in afterAll, and a leftover container is cleaned up + * by the next run reusing the same run-specific container name. + * + * Never throws and never fails its own process: sweeping is opportunistic + * housekeeping run before the required E2E gate (scripts/run-e2e-ci.mjs), + * not part of it. Missing credentials mean "nothing to sweep here", not an + * error -- run-e2e-ci.mjs is what turns missing *required* credentials into + * an explicit failure. + */ + +const MAX_AGE_MS = 24 * 60 * 60 * 1000; +const BRANCH_PREFIX = (provider) => `gfs-e2e-${provider}-`; + +function log(message) { + console.log(`[e2e-sweep] ${message}`); +} + +async function sweepGitHub() { + const owner = process.env.E2E_GITHUB_OWNER; + const repo = process.env.E2E_GITHUB_REPO; + const token = process.env.E2E_GITHUB_TOKEN; + if (!owner || !repo || !token) { + log('github: no credentials configured, skipping'); + return; + } + const headers = { + Authorization: `Bearer ${token}`, + Accept: 'application/vnd.github+json', + 'X-GitHub-Api-Version': '2022-11-28', + }; + const prefix = BRANCH_PREFIX('github'); + + const listRes = await fetch(`https://api.github.com/repos/${owner}/${repo}/branches?per_page=100`, { headers }); + if (!listRes.ok) { + log(`github: failed to list branches (${listRes.status}), skipping`); + return; + } + const branches = await listRes.json(); + + for (const branch of branches) { + if (!branch.name?.startsWith(prefix)) continue; + const commitRes = await fetch(`https://api.github.com/repos/${owner}/${repo}/commits/${branch.commit.sha}`, { headers }); + if (!commitRes.ok) continue; + const commit = await commitRes.json(); + const committedAt = new Date(commit.commit?.committer?.date ?? 0).getTime(); + if (Date.now() - committedAt < MAX_AGE_MS) continue; + + log(`github: deleting stale branch ${branch.name}`); + await fetch(`https://api.github.com/repos/${owner}/${repo}/git/refs/heads/${encodeURIComponent(branch.name)}`, { + method: 'DELETE', + headers, + }).catch(() => {}); + } +} + +async function sweepGitLab() { + const baseUrl = process.env.E2E_GITLAB_BASE_URL ?? 'https://gitlab.com'; + const projectId = process.env.E2E_GITLAB_PROJECT_ID; + const token = process.env.E2E_GITLAB_TOKEN; + if (!projectId || !token) { + log('gitlab: no credentials configured, skipping'); + return; + } + const headers = { 'PRIVATE-TOKEN': token }; + const encodedProjectId = encodeURIComponent(projectId); + const prefix = BRANCH_PREFIX('gitlab'); + + const listRes = await fetch(`${baseUrl}/api/v4/projects/${encodedProjectId}/repository/branches?per_page=100`, { headers }); + if (!listRes.ok) { + log(`gitlab: failed to list branches (${listRes.status}), skipping`); + return; + } + const branches = await listRes.json(); + + for (const branch of branches) { + if (!branch.name?.startsWith(prefix)) continue; + const committedAt = new Date(branch.commit?.committed_date ?? 0).getTime(); + if (Date.now() - committedAt < MAX_AGE_MS) continue; + + log(`gitlab: deleting stale branch ${branch.name}`); + await fetch(`${baseUrl}/api/v4/projects/${encodedProjectId}/repository/branches/${encodeURIComponent(branch.name)}`, { + method: 'DELETE', + headers, + }).catch(() => {}); + } +} + +async function main() { + const providerArg = process.argv.find((arg) => arg.startsWith('--provider=')); + const provider = providerArg?.split('=')[1]; + + const sweeps = { github: sweepGitHub, gitlab: sweepGitLab }; + const toRun = provider ? [provider] : Object.keys(sweeps); + + for (const name of toRun) { + const sweep = sweeps[name]; + if (!sweep) continue; // gitea: no branch sweeper needed, see header comment + try { + await sweep(); + } catch (e) { + log(`${name}: sweep failed, ignoring (best-effort): ${e instanceof Error ? e.message : String(e)}`); + } + } +} + +await main(); diff --git a/scripts/run-e2e-ci.mjs b/scripts/run-e2e-ci.mjs new file mode 100644 index 0000000..b17aa37 --- /dev/null +++ b/scripts/run-e2e-ci.mjs @@ -0,0 +1,50 @@ +#!/usr/bin/env node +/* + * CI entry point for one `provider-e2e` matrix cell (see + * .github/workflows/ci.yml). Thin wrapper around `scripts/run-e2e.mjs` + * (the same command used locally) adding the two things only CI needs: + * + * 1. Sweep stale `gfs-e2e--*` branches first (scripts/e2e-sweep- + * branches.mjs), so a crashed/cancelled prior run's leftover branch + * doesn't linger indefinitely in the sandbox repo/project. + * 2. Fail loudly, not silently, when credentials are missing. + * + * Whether this provider is *supposed* to run at all for the current event + * (e.g. a fork PR only getting Gitea) is decided by the "Determine whether + * this provider leg should run" step in ci.yml (job-level `if:` can't see + * the `matrix` context, so that gate has to be a step, not the job's own + * `if:`) -- by the time this script runs, that gate has already decided + * this cell should execute, so missing credentials here always means + * something is actually broken (an unset repo secret/variable), never "this + * event legitimately has no credentials". A missing required secret must be + * an explicit failure, never a silent skip that reports green. + */ +import { spawnSync } from 'node:child_process'; + +const providerArg = process.argv.find((arg) => arg.startsWith('--provider=')); +const provider = providerArg?.split('=')[1]; + +if (!provider) { + console.error('Usage: node scripts/run-e2e-ci.mjs --provider='); + process.exit(1); +} + +const REQUIRED_ENV = { + github: ['E2E_GITHUB_OWNER', 'E2E_GITHUB_REPO', 'E2E_GITHUB_TOKEN'], + gitlab: ['E2E_GITLAB_PROJECT_ID', 'E2E_GITLAB_TOKEN'], + gitea: [], // provisioned entirely inside the job via Docker; no repo secrets needed +}; + +const missing = (REQUIRED_ENV[provider] ?? []).filter((name) => !process.env[name]); +if (missing.length > 0) { + console.error(`::error::provider-e2e/${provider}: missing required credential(s): ${missing.join(', ')}`); + process.exit(1); +} + +function run(command, args) { + const result = spawnSync(command, args, { stdio: 'inherit' }); + if (result.status !== 0) process.exit(result.status ?? 1); +} + +run('node', ['scripts/e2e-sweep-branches.mjs', `--provider=${provider}`]); +run('node', ['scripts/run-e2e.mjs', '--provider', provider]); diff --git a/scripts/run-e2e.mjs b/scripts/run-e2e.mjs index 144ae48..259c7cb 100644 --- a/scripts/run-e2e.mjs +++ b/scripts/run-e2e.mjs @@ -17,9 +17,18 @@ if (!provider) { const passthrough = args.filter((_, i) => i !== providerIndex && i !== providerIndex + 1); +// Runs the provider's own contract suite plus the shared SyncManager suite +// (parametrized by E2E_PROVIDER, see e2e/suites/sync-manager.e2e.test.ts) in +// the same command/container lifecycle, so one `npm run test:e2e` per +// provider covers both without a second npm script or CI step. const result = spawnSync( 'npx', - ['vitest', 'run', '-c', 'vitest.e2e.config.ts', `e2e/suites/${provider}.e2e.test.ts`, ...passthrough], + [ + 'vitest', 'run', '-c', 'vitest.e2e.config.ts', + `e2e/suites/${provider}.e2e.test.ts`, + 'e2e/suites/sync-manager.e2e.test.ts', + ...passthrough, + ], { stdio: 'inherit', env: { ...process.env, E2E_PROVIDER: provider },