From 9c7d983df46c0cdafd09202026a3c55b736f23e2 Mon Sep 17 00:00:00 2001 From: Nicolas FRADIN Date: Wed, 20 May 2026 23:25:40 +0200 Subject: [PATCH] test: require TEST_PR_ID for write integration tests to prevent accidental mutations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .env.example | 11 +++++ README.md | 13 +++++- tests/integration/bitbucket-api.test.ts | 60 ++++++++++++------------- 3 files changed, 50 insertions(+), 34 deletions(-) create mode 100644 .env.example diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..7afe3fa --- /dev/null +++ b/.env.example @@ -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 diff --git a/README.md b/README.md index ac02861..2aa0e7b 100644 --- a/README.md +++ b/README.md @@ -245,13 +245,13 @@ npm start ### Testing ```bash -# Run tests +# Run all tests (unit only by default) npm test # Run tests in watch mode npm run test:watch -# Integration tests (requires valid credentials) +# Integration tests — read-only, requires valid credentials in .env npm run test:integration # Type check @@ -261,6 +261,15 @@ npx tsc --noEmit 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 ``` diff --git a/tests/integration/bitbucket-api.test.ts b/tests/integration/bitbucket-api.test.ts index 7e836aa..ce15b60 100644 --- a/tests/integration/bitbucket-api.test.ts +++ b/tests/integration/bitbucket-api.test.ts @@ -294,6 +294,7 @@ describe('Error Handling', () => { }); 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)', () => { 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; beforeAll(() => { client = new BitbucketClient(); }); - it('should approve a PR', async () => { + it('should approve then immediately unapprove a PR', async () => { if (!HAS_TOKEN || !RUN_WRITES) return; - const prs = await client.listPullRequests(WORKSPACE, REPO_SLUG, { state: 'OPEN' }); - 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(); - }); + if (!TEST_PR_ID) { console.log('⚠️ TEST_PR_ID not set, skipping approve/unapprove test'); return; } - it('should unapprove a PR', async () => { - if (!HAS_TOKEN || !RUN_WRITES) return; - const prs = await client.listPullRequests(WORKSPACE, REPO_SLUG, { state: 'OPEN' }); - if (prs.length === 0) { console.log('No open PRs, skipping'); return; } - const result = await client.unapprovePullRequest(WORKSPACE, REPO_SLUG, prs[0].id); - expect(result).toHaveProperty('success', true); + await client.approvePullRequest(WORKSPACE, REPO_SLUG, TEST_PR_ID); + try { + const unapproved = await client.unapprovePullRequest(WORKSPACE, REPO_SLUG, TEST_PR_ID); + expect(unapproved).toHaveProperty('success', true); + } finally { + 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; beforeAll(() => { @@ -373,35 +371,33 @@ describe('Comment / Task Write Operations (requires RUN_WRITE_TESTS=true)', () = it('should add, update, then delete a comment', async () => { if (!HAS_TOKEN || !RUN_WRITES) return; - const prs = await client.listPullRequests(WORKSPACE, REPO_SLUG, { state: 'OPEN' }); - if (prs.length === 0) { console.log('No open PRs, skipping'); return; } - const prId = prs[0].id; + if (!TEST_PR_ID) { console.log('⚠️ TEST_PR_ID not set, skipping comment test'); return; } - 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'); const commentId = added.id; - const updated = await client.updatePullRequestComment(WORKSPACE, REPO_SLUG, prId, commentId, { content: 'Updated comment' }); - expect(updated).toHaveProperty('id', commentId); - - const deleted = await client.deletePullRequestComment(WORKSPACE, REPO_SLUG, prId, commentId); - expect(deleted).toHaveProperty('success', true); + try { + const updated = await client.updatePullRequestComment(WORKSPACE, REPO_SLUG, TEST_PR_ID, commentId, { content: 'Updated comment' }); + expect(updated).toHaveProperty('id', commentId); + } finally { + await client.deletePullRequestComment(WORKSPACE, REPO_SLUG, TEST_PR_ID, commentId).catch(() => {}); + } }); it('should create, resolve, then delete a task', async () => { if (!HAS_TOKEN || !RUN_WRITES) return; - const prs = await client.listPullRequests(WORKSPACE, REPO_SLUG, { state: 'OPEN' }); - if (prs.length === 0) { console.log('No open PRs, skipping'); return; } - const prId = prs[0].id; + if (!TEST_PR_ID) { console.log('⚠️ TEST_PR_ID not set, skipping task test'); return; } - 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'); const taskId = created.id; - const resolved = await client.updatePullRequestTask(WORKSPACE, REPO_SLUG, prId, taskId, { state: 'RESOLVED' }); - expect(resolved).toBeDefined(); - - const deleted = await client.deletePullRequestTask(WORKSPACE, REPO_SLUG, prId, taskId); - expect(deleted).toHaveProperty('success', true); + try { + const resolved = await client.updatePullRequestTask(WORKSPACE, REPO_SLUG, TEST_PR_ID, taskId, { state: 'RESOLVED' }); + expect(resolved).toBeDefined(); + } finally { + await client.deletePullRequestTask(WORKSPACE, REPO_SLUG, TEST_PR_ID, taskId).catch(() => {}); + } }); });