jojo/tests/e2e/pr-review.test.e2e.ts
BtbN 6298ee4d3a fix: pull request review comment position (#9914)
## Checklist

This PR contains both #9889 and #9912, since it depends on the one, and the other provides a test for it.
The exact reasoning behind its logic is described here: https://codeberg.org/forgejo/forgejo/issues/9473#issuecomment-7976186

This PR should return the behaviour back to how it was before a PR to Gitea changed it.
Only the resulting Database-Entry will reference the line blamed commit, now also with the correct adjusted line.
While the context diff view is pulled from the commit the commenter actually commented on.

### Tests

- I added test coverage for Go changes...
  - [ ] in their respective `*_test.go` for unit tests.
  - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
  - [ ] in `web_src/js/*.test.js` if it can be unit tested.
  - [x] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.

Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9914
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: BtbN <btbn@btbn.de>
Co-committed-by: BtbN <btbn@btbn.de>
2025-10-31 16:17:23 +01:00

187 lines
8.8 KiB
TypeScript

// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: GPL-3.0-or-later
// @watch start
// templates/repo/diff/new_review.tmpl
// web_src/js/features/repo-issue.js
// @watch end
import {expect} from '@playwright/test';
import {test} from './utils_e2e.ts';
import {screenshot} from './shared/screenshots.ts';
test.use({user: 'user2'});
test('PR: Create review from files', async ({page}) => {
const response = await page.goto('/user2/repo1/pulls/5/files');
expect(response?.status()).toBe(200);
await expect(page.locator('.tippy-box .review-box-panel')).toBeHidden();
await screenshot(page);
// Review panel should appear after clicking Finish review
await page.locator('#review-box .js-btn-review').click();
await expect(page.locator('.tippy-box .review-box-panel')).toBeVisible();
await screenshot(page);
await page.locator('.review-box-panel textarea#_combo_markdown_editor_0')
.fill('This is a review');
await page.locator('.review-box-panel button.btn-submit[value="approve"]').click();
await page.waitForURL(/.*\/user2\/repo1\/pulls\/5#issuecomment-\d+/);
await screenshot(page);
});
test('PR: Create review from commit', async ({page}) => {
const response = await page.goto('/user2/repo1/pulls/3/commits/4a357436d925b5c974181ff12a994538ddc5a269');
expect(response?.status()).toBe(200);
await page.locator('button.add-code-comment').click();
const code_comment = page.locator('.comment-code-cloud form textarea.markdown-text-editor');
await expect(code_comment).toBeVisible();
await code_comment.fill('This is a code comment');
await screenshot(page);
const start_button = page.locator('.comment-code-cloud form button.btn-start-review');
// Workaround for #7152, where there might already be a pending review state from previous
// test runs (most likely to happen when debugging tests).
if (await start_button.isVisible({timeout: 100})) {
await start_button.click();
} else {
await page.locator('.comment-code-cloud form button[name="pending_review"]').click();
}
await expect(page.locator('.comment-list .comment-container')).toBeVisible();
// We need to wait for the review to be processed. Checking the comment counter
// conveniently does that.
await expect(page.locator('#review-box .js-btn-review > span.review-comments-counter')).toHaveText('1');
await page.locator('#review-box .js-btn-review').click();
await expect(page.locator('.tippy-box .review-box-panel')).toBeVisible();
await screenshot(page);
await page.locator('.review-box-panel textarea.markdown-text-editor')
.fill('This is a review');
await page.locator('.review-box-panel button.btn-submit[value="approve"]').click();
await page.waitForURL(/.*\/user2\/repo1\/pulls\/3#issuecomment-\d+/);
await screenshot(page);
// In addition to testing the ability to delete comments, this also
// performs clean up. If tests are run for multiple platforms, the data isn't reset
// in-between, and subsequent runs of this test would fail, because when there already is
// a comment, the on-hover button to start a conversation doesn't appear anymore.
await page.goto('/user2/repo1/pulls/3/commits/4a357436d925b5c974181ff12a994538ddc5a269');
await page.locator('.comment-header-right.actions a.context-menu').click();
await expect(page.locator('.comment-header-right.actions div.menu').getByText(/Copy link.*/)).toBeVisible();
// The button to delete a comment will prompt for confirmation using a browser alert.
page.on('dialog', (dialog) => dialog.accept());
await page.locator('.comment-header-right.actions div.menu .delete-comment').click();
await expect(page.locator('.comment-list .comment-container')).toBeHidden();
await screenshot(page);
});
test('PR: Navigate by single commit', async ({page}) => {
const response = await page.goto('/user2/repo1/pulls/3/commits');
expect(response?.status()).toBe(200);
await page.locator('tbody.commit-list td.message a').nth(1).click();
await page.waitForURL(/.*\/user2\/repo1\/pulls\/3\/commits\/4a357436d925b5c974181ff12a994538ddc5a269/);
await screenshot(page);
let prevButton = page.locator('.commit-header-buttons').getByText(/Prev/);
let nextButton = page.locator('.commit-header-buttons').getByText(/Next/);
await prevButton.waitFor();
await nextButton.waitFor();
await expect(prevButton).toHaveClass(/disabled/);
await expect(nextButton).not.toHaveClass(/disabled/);
await expect(nextButton).toHaveAttribute('href', '/user2/repo1/pulls/3/commits/5f22f7d0d95d614d25a5b68592adb345a4b5c7fd');
await nextButton.click();
await page.waitForURL(/.*\/user2\/repo1\/pulls\/3\/commits\/5f22f7d0d95d614d25a5b68592adb345a4b5c7fd/);
await screenshot(page);
prevButton = page.locator('.commit-header-buttons').getByText(/Prev/);
nextButton = page.locator('.commit-header-buttons').getByText(/Next/);
await prevButton.waitFor();
await nextButton.waitFor();
await expect(prevButton).not.toHaveClass(/disabled/);
await expect(nextButton).toHaveClass(/disabled/);
await expect(prevButton).toHaveAttribute('href', '/user2/repo1/pulls/3/commits/4a357436d925b5c974181ff12a994538ddc5a269');
});
test('PR: Test mentions values', async ({page}) => {
const response = await page.goto('/user2/repo1/pulls/5/files');
expect(response?.status()).toBe(200);
await page.locator('#review-box .js-btn-review').click();
await expect(page.locator('.tippy-box .review-box-panel')).toBeVisible();
await page.locator('.review-box-panel textarea#_combo_markdown_editor_0')
.fill('@');
await screenshot(page);
await expect(page.locator('ul.suggestions li span:first-of-type')).toContainText([
'user1',
'user2',
]);
await page.locator("ul.suggestions li[data-value='@user1']").click();
await expect(page.locator('.review-box-panel textarea#_combo_markdown_editor_0')).toHaveValue('@user1 ');
});
test('multi-commit commenting', async ({page, request}) => {
const response = await page.goto('/user2/long-diff-test');
expect(response?.status()).toBe(200);
try {
await page.getByText('2 branches').click(); // navigate to branch list
await page.getByText('New pull request').click(); // load compare view for the branch
await page.locator('.show-form-container').getByText('New pull request').click(); // actually open the PR form
await page.locator('.primary.button').getByText('Create pull request').click(); // submit PR creation
// Test situation: adding a comment on a line that was created in the *second* commit, doing it from the "Files changed" view.
await page.getByText('Files changed').click();
await page.getByText('More This line was changed in commit 2')
.locator('..')
.locator('button.add-code-comment')
.click();
await page.getByPlaceholder('Leave a comment').fill('Comment on line changed in commit 2');
await page.getByText('Add single comment').click();
// Test assertion: when viewing the comment from the 'Conversation' page, it's diff should look correct:
await page.getByText('Conversation').click();
await expect(page.locator('.pull.menu .item.active')).toContainText('Conversation'); // ensure we navigated back to Conversation page
await expect(page.locator('.text.comment-content .render-content.markup')).toHaveText('Comment on line changed in commit 2');
await expect(page.locator('.diff-file-box .code-diff')).toContainText('More This line was changed in commit 2');
// Test assertion: when viewing the comment from the second commit, it should be placed correctly in the UI:
await page.getByRole('link', {name: 'Commits'}).click();
await page.getByText('add commit to branch').nth(1).click();
// FIXME: The intent of this test is to make sure that the comment box appears in the "right spot", which is *below*
// the line of code that it was commented on. This check uses the elements bounding boxes which... is pretty ugly
// and could be done better. Probably would be better to find the line of code that the box is rendered on,
// instead.
const codeLine = await page.getByText('More This line was changed in commit 2').boundingBox();
const commentBox = await page.locator('.render-content.markup').getByText('Comment on line changed in commit 2').boundingBox();
expect(commentBox.y).toBeGreaterThan(codeLine.y);
} finally {
// Delete any PRs on the test repo so that this test can be rerun.
const issuesResp = await request.get(`/api/v1/repos/user2/long-diff-test/issues`);
expect(issuesResp.ok()).toBeTruthy();
const issues = await issuesResp.json();
for (const issue of issues) {
const delResp = await request.delete(`/api/v1/repos/user2/long-diff-test/issues/${issue.number}`, {
headers: {
'Content-Type': 'application/json',
'Authorization': `Basic ${btoa(`user1:password`)}`,
},
});
expect(delResp.ok()).toBeTruthy();
}
}
});