| ... | @@ -9,6 +9,7 @@ import * as Ref from "effect/Ref"; | ... | @@ -9,6 +9,7 @@ import * as Ref from "effect/Ref"; |
| 9 | import * as Schedule from "effect/Schedule"; | 9 | import * as Schedule from "effect/Schedule"; |
| 10 | import { | 10 | import { |
| 11 | ProjectId, | 11 | ProjectId, |
| | 12 | GitCommandError, |
| 12 | type PullRequestActivity, | 13 | type PullRequestActivity, |
| 13 | type PullRequestDetail, | 14 | type PullRequestDetail, |
| 14 | type PullRequestStack, | 15 | type PullRequestStack, |
| ... | @@ -144,7 +145,7 @@ const stack = (repositories: { | ... | @@ -144,7 +145,7 @@ const stack = (repositories: { |
| 144 | { | 145 | { |
| 145 | number: 8, | 146 | number: 8, |
| 146 | title: "Upper", | 147 | title: "Upper", |
| 147 | headSha: repositories.upperSha, | 148 | headSha: repositories.upperSha.slice(0, 12), |
| 148 | headBranch: "upper", | 149 | headBranch: "upper", |
| 149 | state: "open", | 150 | state: "open", |
| 150 | }, | 151 | }, |
| ... | @@ -228,8 +229,328 @@ const layerBase = Layer.mergeAll( | ... | @@ -228,8 +229,328 @@ const layerBase = Layer.mergeAll( |
| 228 | SqlitePersistence.layerMemory, | 229 | SqlitePersistence.layerMemory, |
| 229 | ).pipe(Layer.provideMerge(NodeServices.layer)); | 230 | ).pipe(Layer.provideMerge(NodeServices.layer)); |
| 230 | | 231 | |
| | 232 | type Repositories = Effect.Success<typeof makeRepositories>; |
| | 233 | |
| | 234 | const awaitRunFailure = ( |
| | 235 | service: PullRequestReviewGuideService.PullRequestReviewGuideService["Service"], |
| | 236 | input: PullRequestRef, |
| | 237 | ) => |
| | 238 | service.get(input).pipe( |
| | 239 | Effect.flatMap((result) => |
| | 240 | result.run?._tag === "Failed" |
| | 241 | ? Effect.succeed(result.run.detail) |
| | 242 | : Effect.fail("still running" as const), |
| | 243 | ), |
| | 244 | Effect.retry(Schedule.spaced("20 millis")), |
| | 245 | ); |
| | 246 | |
| | 247 | /** A workspace cloned with only `main`, so the change's head is not there until fetched. */ |
| | 248 | const startSingleBranchRun = (input: { |
| | 249 | readonly repositoryName: string; |
| | 250 | readonly reportedHead: (repositories: Repositories) => string; |
| | 251 | readonly refuseCommitFetch: boolean; |
| | 252 | readonly prepare: ( |
| | 253 | repositories: Repositories, |
| | 254 | workspace: string, |
| | 255 | ) => Effect.Effect<void, GitCommandError, GitVcsDriver.GitVcsDriver>; |
| | 256 | /** Runs inside the head branch's fetch, after the service asked for it and before git runs. */ |
| | 257 | readonly onBranchFetch?: |
| | 258 | | (( |
| | 259 | repositories: Repositories, |
| | 260 | ) => Effect.Effect<void, GitCommandError, GitVcsDriver.GitVcsDriver>) |
| | 261 | | undefined; |
| | 262 | }) => |
| | 263 | Effect.gen(function* () { |
| | 264 | const repositories = yield* makeRepositories; |
| | 265 | const workspace = `${repositories.root}/single-branch`; |
| | 266 | yield* git(repositories.root, [ |
| | 267 | "clone", |
| | 268 | "--quiet", |
| | 269 | "--no-local", |
| | 270 | "--single-branch", |
| | 271 | "--branch", |
| | 272 | "main", |
| | 273 | `${repositories.root}/remote`, |
| | 274 | workspace, |
| | 275 | ]); |
| | 276 | yield* input.prepare(repositories, workspace); |
| | 277 | const reportedHead = input.reportedHead(repositories); |
| | 278 | const stored = yield* Deferred.make<void>(); |
| | 279 | const driver = yield* GitVcsDriver.GitVcsDriver; |
| | 280 | const fetchedRefs: Array<string> = []; |
| | 281 | const observed: Array<{ cwd: string; head: string; changed: string }> = []; |
| | 282 | const onBranchFetch = input.onBranchFetch; |
| | 283 | const observedDriver = { |
| | 284 | ...driver, |
| | 285 | execute: (call: GitVcsDriver.ExecuteGitInput) => |
| | 286 | Effect.gen(function* () { |
| | 287 | const ref = call.args.at(-1); |
| | 288 | if (call.args[0] !== "fetch" || ref === undefined) return yield* driver.execute(call); |
| | 289 | fetchedRefs.push(ref); |
| | 290 | if (input.refuseCommitFetch && ref === repositories.headSha) { |
| | 291 | return yield* new GitCommandError({ |
| | 292 | operation: call.operation, |
| | 293 | command: "git", |
| | 294 | cwd: call.cwd, |
| | 295 | detail: "The test remote refuses fetches by commit.", |
| | 296 | exitCode: 128, |
| | 297 | }); |
| | 298 | } |
| | 299 | if (ref === "refs/heads/feature" && onBranchFetch !== undefined) { |
| | 300 | yield* onBranchFetch(repositories).pipe( |
| | 301 | Effect.provideService(GitVcsDriver.GitVcsDriver, driver), |
| | 302 | ); |
| | 303 | } |
| | 304 | return yield* driver.execute(call); |
| | 305 | }), |
| | 306 | }; |
| | 307 | const readCheckout = (cwd: string, args: ReadonlyArray<string>) => |
| | 308 | driver |
| | 309 | .execute({ operation: "test.readCheckout", cwd, args }) |
| | 310 | .pipe(Effect.map((result) => result.stdout.trim())); |
| | 311 | const context = yield* Layer.build( |
| | 312 | makeLayer({ |
| | 313 | ...repositories, |
| | 314 | headSha: reportedHead, |
| | 315 | workspace, |
| | 316 | stored, |
| | 317 | identity: { |
| | 318 | ...ACME_APP, |
| | 319 | canonicalKey: `github.com/acme/${input.repositoryName}`, |
| | 320 | displayName: `acme/${input.repositoryName}`, |
| | 321 | name: input.repositoryName, |
| | 322 | }, |
| | 323 | generate: (generation) => |
| | 324 | Effect.gen(function* () { |
| | 325 | observed.push({ |
| | 326 | cwd: generation.cwd, |
| | 327 | head: yield* readCheckout(generation.cwd, ["rev-parse", "HEAD"]), |
| | 328 | changed: yield* readCheckout(generation.cwd, [ |
| | 329 | "diff", |
| | 330 | "--name-only", |
| | 331 | `${generation.baseCommit}...HEAD`, |
| | 332 | ]), |
| | 333 | }); |
| | 334 | return { stops: [] }; |
| | 335 | }).pipe(Effect.orDie), |
| | 336 | }).pipe(Layer.provide(Layer.succeed(GitVcsDriver.GitVcsDriver, observedDriver))), |
| | 337 | ); |
| | 338 | const service = Context.get( |
| | 339 | context, |
| | 340 | PullRequestReviewGuideService.PullRequestReviewGuideService, |
| | 341 | ); |
| | 342 | const guideReference = { ...reference, repository: `acme/${input.repositoryName}` }; |
| | 343 | yield* service.generate(guideReference); |
| | 344 | return { |
| | 345 | repositories, |
| | 346 | reportedHead, |
| | 347 | service, |
| | 348 | reference: guideReference, |
| | 349 | stored, |
| | 350 | fetchedRefs, |
| | 351 | observed, |
| | 352 | }; |
| | 353 | }); |
| | 354 | |
| 231 | // Runs drive real git, which the test clock does not advance, so waits use real time. | 355 | // Runs drive real git, which the test clock does not advance, so waits use real time. |
| 232 | it.layer(layerBase, { excludeTestServices: true })("PullRequestReviewGuideService", (it) => { | 356 | it.layer(layerBase, { excludeTestServices: true })("PullRequestReviewGuideService", (it) => { |
| | 357 | it.effect.each([ |
| | 358 | { |
| | 359 | name: "a short head already fetched", |
| | 360 | shortHash: true, |
| | 361 | prepare: "fetch-feature", |
| | 362 | pushDuringFetch: false, |
| | 363 | refuseCommitFetch: true, |
| | 364 | expectedFetches: [], |
| | 365 | }, |
| | 366 | { |
| | 367 | name: "a short head on its branch", |
| | 368 | shortHash: true, |
| | 369 | prepare: "none", |
| | 370 | pushDuringFetch: false, |
| | 371 | refuseCommitFetch: true, |
| | 372 | expectedFetches: ["refs/heads/feature"], |
| | 373 | }, |
| | 374 | { |
| | 375 | name: "a short head whose branch moves during the fetch", |
| | 376 | shortHash: true, |
| | 377 | prepare: "none", |
| | 378 | pushDuringFetch: true, |
| | 379 | refuseCommitFetch: true, |
| | 380 | expectedFetches: ["refs/heads/feature"], |
| | 381 | }, |
| | 382 | { |
| | 383 | name: "a full head served by commit", |
| | 384 | shortHash: false, |
| | 385 | prepare: "none", |
| | 386 | pushDuringFetch: false, |
| | 387 | refuseCommitFetch: false, |
| | 388 | expectedFetches: ["<head>"], |
| | 389 | }, |
| | 390 | { |
| | 391 | name: "a full head on its branch", |
| | 392 | shortHash: false, |
| | 393 | prepare: "none", |
| | 394 | pushDuringFetch: false, |
| | 395 | refuseCommitFetch: true, |
| | 396 | expectedFetches: ["<head>", "refs/heads/feature"], |
| | 397 | }, |
| | 398 | { |
| | 399 | name: "a full head only on its pull ref", |
| | 400 | shortHash: false, |
| | 401 | prepare: "refs/pull/7/head", |
| | 402 | pushDuringFetch: false, |
| | 403 | refuseCommitFetch: true, |
| | 404 | expectedFetches: ["<head>", "refs/heads/feature", "refs/pull/7/head"], |
| | 405 | }, |
| | 406 | { |
| | 407 | name: "a full head only on its merge request ref", |
| | 408 | shortHash: false, |
| | 409 | prepare: "refs/merge-requests/7/head", |
| | 410 | pushDuringFetch: false, |
| | 411 | refuseCommitFetch: true, |
| | 412 | expectedFetches: [ |
| | 413 | "<head>", |
| | 414 | "refs/heads/feature", |
| | 415 | "refs/pull/7/head", |
| | 416 | "refs/merge-requests/7/head", |
| | 417 | ], |
| | 418 | }, |
| | 419 | ])( |
| | 420 | "writes a guide for the reported commit: $name", |
| | 421 | ({ shortHash, prepare, pushDuringFetch, refuseCommitFetch, expectedFetches }) => |
| | 422 | Effect.scoped( |
| | 423 | Effect.gen(function* () { |
| | 424 | const run = yield* startSingleBranchRun({ |
| | 425 | repositoryName: "fetched-head", |
| | 426 | reportedHead: (repositories) => |
| | 427 | shortHash ? repositories.headSha.slice(0, 12) : repositories.headSha, |
| | 428 | refuseCommitFetch, |
| | 429 | prepare: (repositories, workspace) => { |
| | 430 | const remote = `${repositories.root}/remote`; |
| | 431 | if (prepare === "fetch-feature") { |
| | 432 | return git(workspace, ["fetch", "--quiet", "origin", "feature"]).pipe( |
| | 433 | Effect.asVoid, |
| | 434 | ); |
| | 435 | } |
| | 436 | if (prepare === "none") return Effect.void; |
| | 437 | return git(remote, ["update-ref", prepare, repositories.headSha]).pipe( |
| | 438 | Effect.andThen( |
| | 439 | git(remote, ["update-ref", "refs/heads/feature", repositories.lowerSha]), |
| | 440 | ), |
| | 441 | Effect.asVoid, |
| | 442 | ); |
| | 443 | }, |
| | 444 | onBranchFetch: pushDuringFetch |
| | 445 | ? (repositories) => { |
| | 446 | const remote = `${repositories.root}/remote`; |
| | 447 | return git(remote, ["checkout", "--quiet", "feature"]).pipe( |
| | 448 | Effect.andThen( |
| | 449 | git(remote, [ |
| | 450 | "commit", |
| | 451 | "--quiet", |
| | 452 | "--allow-empty", |
| | 453 | "-m", |
| | 454 | "push during fetch", |
| | 455 | ]), |
| | 456 | ), |
| | 457 | Effect.asVoid, |
| | 458 | ); |
| | 459 | } |
| | 460 | : undefined, |
| | 461 | }); |
| | 462 | yield* Effect.raceFirst( |
| | 463 | Deferred.await(run.stored), |
| | 464 | awaitRunFailure(run.service, run.reference).pipe( |
| | 465 | Effect.flatMap((detail) => Effect.die(`run failed: ${detail}`)), |
| | 466 | ), |
| | 467 | ); |
| | 468 | const { headSha } = run.repositories; |
| | 469 | const result = yield* run.service.get(run.reference); |
| | 470 | assert.strictEqual(result.guide?.headSha, run.reportedHead); |
| | 471 | assert.deepStrictEqual(result.guide?.stops[0]?.files, [{ path: "feature.txt" }]); |
| | 472 | assert.deepStrictEqual(run.observed, [ |
| | 473 | { cwd: run.observed[0]?.cwd, head: headSha, changed: "feature.txt" }, |
| | 474 | ]); |
| | 475 | const headRefs = new Set([ |
| | 476 | headSha, |
| | 477 | run.reportedHead, |
| | 478 | "refs/heads/feature", |
| | 479 | "refs/pull/7/head", |
| | 480 | "refs/merge-requests/7/head", |
| | 481 | ]); |
| | 482 | assert.deepStrictEqual( |
| | 483 | run.fetchedRefs.filter((ref) => headRefs.has(ref)), |
| | 484 | expectedFetches.map((ref) => (ref === "<head>" ? headSha : ref)), |
| | 485 | ); |
| | 486 | const fs = yield* FileSystem.FileSystem; |
| | 487 | assert.isFalse(yield* fs.exists(run.observed[0]?.cwd ?? "")); |
| | 488 | }), |
| | 489 | ), |
| | 490 | ); |
| | 491 | |
| | 492 | it.effect.each([ |
| | 493 | { |
| | 494 | name: "a branch named like the short head", |
| | 495 | reportedHead: (repositories: Repositories) => repositories.headSha.slice(0, 12), |
| | 496 | shadow: "branch", |
| | 497 | expected: ["a branch or tag rather than the commit"], |
| | 498 | }, |
| | 499 | { |
| | 500 | name: "a tag named like the short head", |
| | 501 | reportedHead: (repositories: Repositories) => repositories.headSha.slice(0, 12), |
| | 502 | shadow: "tag", |
| | 503 | expected: ["a branch or tag rather than the commit"], |
| | 504 | }, |
| | 505 | { |
| | 506 | name: "a head that is not a commit hash", |
| | 507 | reportedHead: (repositories: Repositories) => |
| | 508 | `--upload-pack=touch ${repositories.root}/injected`, |
| | 509 | shadow: "none", |
| | 510 | expected: ["not a commit hash"], |
| | 511 | }, |
| | 512 | { |
| | 513 | name: "a head no source has", |
| | 514 | reportedHead: () => "e".repeat(40), |
| | 515 | shadow: "none", |
| | 516 | expected: [ |
| | 517 | "local objects:", |
| | 518 | "the commit:", |
| | 519 | "branch feature:", |
| | 520 | "refs/pull/7/head:", |
| | 521 | "refs/merge-requests/7/head:", |
| | 522 | ], |
| | 523 | }, |
| | 524 | ])("refuses to write a guide for $name", ({ reportedHead, shadow, expected }) => |
| | 525 | Effect.scoped( |
| | 526 | Effect.gen(function* () { |
| | 527 | const run = yield* startSingleBranchRun({ |
| | 528 | repositoryName: "refused-head", |
| | 529 | reportedHead, |
| | 530 | refuseCommitFetch: false, |
| | 531 | prepare: (repositories, workspace) => |
| | 532 | git(workspace, ["fetch", "--quiet", "origin", "feature"]).pipe( |
| | 533 | Effect.andThen( |
| | 534 | shadow === "none" |
| | 535 | ? Effect.void |
| | 536 | : git(workspace, [shadow, repositories.headSha.slice(0, 12), "HEAD"]), |
| | 537 | ), |
| | 538 | Effect.asVoid, |
| | 539 | ), |
| | 540 | }); |
| | 541 | const detail = yield* Effect.raceFirst( |
| | 542 | awaitRunFailure(run.service, run.reference), |
| | 543 | Deferred.await(run.stored).pipe(Effect.andThen(Effect.die("a guide was written"))), |
| | 544 | ); |
| | 545 | for (const part of expected) assert.include(detail, part); |
| | 546 | assert.deepStrictEqual(run.observed, []); |
| | 547 | assert.isFalse(run.fetchedRefs.some((ref) => ref.startsWith("-"))); |
| | 548 | const fs = yield* FileSystem.FileSystem; |
| | 549 | assert.isFalse(yield* fs.exists(`${run.repositories.root}/injected`)); |
| | 550 | }), |
| | 551 | ), |
| | 552 | ); |
| | 553 | |
| 233 | it.effect("briefs the change against its own base in a checkout it then removes", () => | 554 | it.effect("briefs the change against its own base in a checkout it then removes", () => |
| 234 | Effect.scoped( | 555 | Effect.scoped( |
| 235 | Effect.gen(function* () { | 556 | Effect.gen(function* () { |
Paweł Mikołajczuk <pawel@mikolajczuk.in> 2026-10-13 16:25:11+11:00
GitHub <noreply@github.com> 2026-10-13 00:25:11-05:00