test: require TEST_PR_ID for write integration tests to prevent accidental mutations

Write tests no longer pick a random open PR. RUN_WRITE_TESTS=true alone is not
sufficient — TEST_PR_ID must also be set, otherwise each write test is skipped
with a warning. Added try/finally cleanup blocks so approval/comment/task state
is always restored even on assertion failures. Updated .env.example and README
to document the new requirement.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-20 23:25:40 +02:00
parent ab73c92e6d
commit 9c7d983df4
3 changed files with 50 additions and 34 deletions

11
.env.example Normal file
View File

@@ -0,0 +1,11 @@
BITBUCKET_MCP_EMAIL=your_email@example.com
BITBUCKET_MCP_TOKEN=your_api_token
DEFAULT_WORKSPACE=your-workspace
DEFAULT_REPO=your-repo
# Integration test write operations
# Set RUN_WRITE_TESTS=true to enable tests that create/modify/delete data.
# TEST_PR_ID must also be set — write tests will only run against this specific PR
# to avoid accidentally modifying unrelated pull requests.
# RUN_WRITE_TESTS=true
# TEST_PR_ID=123

View File

@@ -245,13 +245,13 @@ npm start
### Testing ### Testing
```bash ```bash
# Run tests # Run all tests (unit only by default)
npm test npm test
# Run tests in watch mode # Run tests in watch mode
npm run test:watch npm run test:watch
# Integration tests (requires valid credentials) # Integration tests — read-only, requires valid credentials in .env
npm run test:integration npm run test:integration
# Type check # Type check
@@ -261,6 +261,15 @@ npx tsc --noEmit
npm run test:coverage npm run test:coverage
``` ```
**Write operation tests** (approve/unapprove, comments, tasks) are disabled by default. To enable them, set two variables in your `.env`:
```env
RUN_WRITE_TESTS=true
TEST_PR_ID=123 # ID of a PR you own and can safely modify
```
`TEST_PR_ID` is required — write tests will not fall back to a random PR. If it is not set, each write test is skipped with a warning. This prevents accidental modifications to production PRs.
### Project Structure ### Project Structure
``` ```

View File

@@ -294,6 +294,7 @@ describe('Error Handling', () => {
}); });
const RUN_WRITES = process.env.RUN_WRITE_TESTS === 'true'; const RUN_WRITES = process.env.RUN_WRITE_TESTS === 'true';
const TEST_PR_ID = process.env.TEST_PR_ID ? parseInt(process.env.TEST_PR_ID, 10) : undefined;
describe('Repository / Workspace (read)', () => { describe('Repository / Workspace (read)', () => {
let client: BitbucketClient; let client: BitbucketClient;
@@ -340,31 +341,28 @@ describe('Repository / Workspace (read)', () => {
}); });
}); });
describe('PR Write Operations (requires RUN_WRITE_TESTS=true)', () => { describe('PR Write Operations (requires RUN_WRITE_TESTS=true and TEST_PR_ID)', () => {
let client: BitbucketClient; let client: BitbucketClient;
beforeAll(() => { beforeAll(() => {
client = new BitbucketClient(); client = new BitbucketClient();
}); });
it('should approve a PR', async () => { it('should approve then immediately unapprove a PR', async () => {
if (!HAS_TOKEN || !RUN_WRITES) return; if (!HAS_TOKEN || !RUN_WRITES) return;
const prs = await client.listPullRequests(WORKSPACE, REPO_SLUG, { state: 'OPEN' }); if (!TEST_PR_ID) { console.log('⚠️ TEST_PR_ID not set, skipping approve/unapprove test'); return; }
if (prs.length === 0) { console.log('No open PRs, skipping'); return; }
const result = await client.approvePullRequest(WORKSPACE, REPO_SLUG, prs[0].id);
expect(result).toBeDefined();
});
it('should unapprove a PR', async () => { await client.approvePullRequest(WORKSPACE, REPO_SLUG, TEST_PR_ID);
if (!HAS_TOKEN || !RUN_WRITES) return; try {
const prs = await client.listPullRequests(WORKSPACE, REPO_SLUG, { state: 'OPEN' }); const unapproved = await client.unapprovePullRequest(WORKSPACE, REPO_SLUG, TEST_PR_ID);
if (prs.length === 0) { console.log('No open PRs, skipping'); return; } expect(unapproved).toHaveProperty('success', true);
const result = await client.unapprovePullRequest(WORKSPACE, REPO_SLUG, prs[0].id); } finally {
expect(result).toHaveProperty('success', true); await client.unapprovePullRequest(WORKSPACE, REPO_SLUG, TEST_PR_ID).catch(() => {});
}
}); });
}); });
describe('Comment / Task Write Operations (requires RUN_WRITE_TESTS=true)', () => { describe('Comment / Task Write Operations (requires RUN_WRITE_TESTS=true and TEST_PR_ID)', () => {
let client: BitbucketClient; let client: BitbucketClient;
beforeAll(() => { beforeAll(() => {
@@ -373,35 +371,33 @@ describe('Comment / Task Write Operations (requires RUN_WRITE_TESTS=true)', () =
it('should add, update, then delete a comment', async () => { it('should add, update, then delete a comment', async () => {
if (!HAS_TOKEN || !RUN_WRITES) return; if (!HAS_TOKEN || !RUN_WRITES) return;
const prs = await client.listPullRequests(WORKSPACE, REPO_SLUG, { state: 'OPEN' }); if (!TEST_PR_ID) { console.log('⚠️ TEST_PR_ID not set, skipping comment test'); return; }
if (prs.length === 0) { console.log('No open PRs, skipping'); return; }
const prId = prs[0].id;
const added = await client.addPullRequestComment(WORKSPACE, REPO_SLUG, prId, { content: 'Integration test comment' }); const added = await client.addPullRequestComment(WORKSPACE, REPO_SLUG, TEST_PR_ID, { content: 'Integration test comment' });
expect(added).toHaveProperty('id'); expect(added).toHaveProperty('id');
const commentId = added.id; const commentId = added.id;
const updated = await client.updatePullRequestComment(WORKSPACE, REPO_SLUG, prId, commentId, { content: 'Updated comment' }); try {
expect(updated).toHaveProperty('id', commentId); const updated = await client.updatePullRequestComment(WORKSPACE, REPO_SLUG, TEST_PR_ID, commentId, { content: 'Updated comment' });
expect(updated).toHaveProperty('id', commentId);
const deleted = await client.deletePullRequestComment(WORKSPACE, REPO_SLUG, prId, commentId); } finally {
expect(deleted).toHaveProperty('success', true); await client.deletePullRequestComment(WORKSPACE, REPO_SLUG, TEST_PR_ID, commentId).catch(() => {});
}
}); });
it('should create, resolve, then delete a task', async () => { it('should create, resolve, then delete a task', async () => {
if (!HAS_TOKEN || !RUN_WRITES) return; if (!HAS_TOKEN || !RUN_WRITES) return;
const prs = await client.listPullRequests(WORKSPACE, REPO_SLUG, { state: 'OPEN' }); if (!TEST_PR_ID) { console.log('⚠️ TEST_PR_ID not set, skipping task test'); return; }
if (prs.length === 0) { console.log('No open PRs, skipping'); return; }
const prId = prs[0].id;
const created = await client.createPullRequestTask(WORKSPACE, REPO_SLUG, prId, { content: 'Integration test task' }); const created = await client.createPullRequestTask(WORKSPACE, REPO_SLUG, TEST_PR_ID, { content: 'Integration test task' });
expect(created).toHaveProperty('id'); expect(created).toHaveProperty('id');
const taskId = created.id; const taskId = created.id;
const resolved = await client.updatePullRequestTask(WORKSPACE, REPO_SLUG, prId, taskId, { state: 'RESOLVED' }); try {
expect(resolved).toBeDefined(); const resolved = await client.updatePullRequestTask(WORKSPACE, REPO_SLUG, TEST_PR_ID, taskId, { state: 'RESOLVED' });
expect(resolved).toBeDefined();
const deleted = await client.deletePullRequestTask(WORKSPACE, REPO_SLUG, prId, taskId); } finally {
expect(deleted).toHaveProperty('success', true); await client.deletePullRequestTask(WORKSPACE, REPO_SLUG, TEST_PR_ID, taskId).catch(() => {});
}
}); });
}); });