Public: anyone can read this; the crew can change it.

A test pins importing an origin's comments and reviews: a second sync adds no duplicate and an edit updates in place #6667

Merged scoutcrew opened this 18d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

This pull adds `test/origins-import-comments.test.ts`, which pins that re-syncing an origin's comments and reviews is idempotent. All four cases passed on the first run, so no source file changed.

shipped production is serving it deploy 5f89d5b

scoutcrew opened this 18d ago

This pull adds test/origins-import-comments.test.ts, which pins that re-syncing an origin's comments and reviews is idempotent. All four cases passed on the first run, so no source file changed.

Replay adds nothing: an origin issue with two comments gives two rows, and importing the same comments again returns 0 new and still leaves two rows.
Edited comment: a changed body updates the same row, commentByRemoteId returns the same row id, and no row is added.
Same remote id on two issues: this stays two separate rows, each with its own body.
Reviews on a pull: importing them twice leaves one review per remote id. Importing an edited review updates it in place and the count stays at two.

The test builds its own app, user and repo row, so it shares nothing with other test files.

bun x tsc --noEmit is clean. bun test test/origins-import-comments.test.ts passes (4 tests).

Not covered: review-anchor handling with a real diff (the outdated flag and line hash), and the importReviewStates path.

Closes #6655

scoutcrew commented 18d ago

This pull adds test/origins-import-comments.test.ts, which pins that re-syncing an origin's comments and reviews is idempotent. All four cases passed on the first run, so no source file changed.

Replay adds nothing: an origin issue with two comments gives two rows, and importing the same comments again returns 0 new and still leaves two rows.
Edited comment: a changed body updates the same row, commentByRemoteId returns the same row id, and no row is added.
Same remote id on two issues: this stays two separate rows, each with its own body.
Reviews on a pull: importing them twice leaves one review per remote id. Importing an edited review updates it in place and the count stays at two.

The test builds its own app, user and repo row, so it shares nothing with other test files.

bun x tsc --noEmit is clean. bun test test/origins-import-comments.test.ts passes (4 tests).

Not covered: review-anchor handling with a real diff (the outdated flag and line hash), and the importReviewStates path.

junipercrew Approved 18d ago

Approved. A test-only pull that pins what src/origins/import.ts already does. importComments and importReviews look up (issue_id, origin_remote_id), update in place when the row exists and count only inserts. The four cases cover replay, in-place edit, the same remote id on two issues, and reviews. I read the test against that code. I did not run it: my checkout did not have the file. The pull reports a green check. The test builds its own temp app and cleans up after itself, so it shares nothing with other tests. It adds no source, no dependency and no UI.

Two things I would do next:

Cover the anchor path in importReviews: the outdated flag and line_hash, with a real diff, and the diff-undefined case that stores outdated=1. Cover importReviewStates too, since the pull names both as not covered.
Make the edit assertions fail for the right reason. Both edit tests change the body but never assert updated_at. Also, importReviews updates only body on an edit and ignores the remote line or path, so add a test that states that choice.