-
Notifications
You must be signed in to change notification settings - Fork 21
Add e2e tests for cloudflare service (basic) #304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #304 +/- ##
=======================================
Coverage 59.31% 59.31%
=======================================
Files 44 44
Lines 5110 5110
Branches 383 383
=======================================
Hits 3031 3031
Misses 2079 2079
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…in permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Coverage Initialization and Data Aggregation Issues
This test suite exhibits two coverage-related bugs:
- Duplicate Coverage Initialization: Coverage collection is started redundantly in the
beforeEach
hook ofsimple.spec.ts
, as it's already initiated globally incoverage.setup.ts
. This causes Playwright to throw an error because coverage cannot be started twice on the same page instance. - Race Condition in Coverage Data Aggregation: The
jsCoverage
andcssCoverage
arrays are shared and accessed concurrently by multiple tests whenfullyParallel: true
is enabled. This race condition can lead to data corruption or loss in the aggregated coverage reports.
packages/mcp-cloudflare-e2e/tests/simple.spec.ts#L4-L21
sentry-mcp/packages/mcp-cloudflare-e2e/tests/simple.spec.ts
Lines 4 to 21 in 350d990
test.describe("Simple Application Tests", () => { | |
const jsCoverage: any[] = []; | |
const cssCoverage: any[] = []; | |
test.beforeEach(async ({ page }) => { | |
// Start coverage collection for each test | |
await page.coverage.startJSCoverage(); | |
await page.coverage.startCSSCoverage(); | |
}); | |
test.afterEach(async ({ page }) => { | |
// Collect coverage after each test | |
const jsResults = await page.coverage.stopJSCoverage(); | |
const cssResults = await page.coverage.stopCSSCoverage(); | |
jsCoverage.push(...jsResults); | |
cssCoverage.push(...cssResults); | |
}); |
Bug: Incompatible Coverage Tools Conflict
The test:coverage
script in packages/mcp-cloudflare-e2e/package.json
attempts to use nyc
for coverage reporting. This is incompatible as nyc
processes Node.js coverage, while Playwright collects browser-based coverage, leading to script failure or incorrect reports.
packages/mcp-cloudflare-e2e/package.json#L1-L22
sentry-mcp/packages/mcp-cloudflare-e2e/package.json
Lines 1 to 22 in 350d990
{ | |
"name": "@sentry/mcp-cloudflare-e2e", | |
"version": "0.12.0", | |
"private": true, | |
"description": "End-to-end tests for Sentry MCP", | |
"scripts": { | |
"test": "playwright test", | |
"test:coverage": "mkdir -p coverage && playwright test && nyc report --reporter=lcov --reporter=json --reporter=text", | |
"test:ci": "mkdir -p coverage && playwright test --reporter=junit", | |
"test:headed": "playwright test --headed", | |
"test:ui": "playwright test --ui", | |
"test:debug": "playwright test --debug", | |
"show-report": "playwright show-report", | |
"install:browsers": "playwright install --with-deps" | |
}, | |
"devDependencies": { | |
"@playwright/test": "^1.43.0", | |
"typescript": "^5.4.5", | |
"nyc": "^17.1.0", | |
"@types/node": "^22.10.6" | |
} | |
} |
Was this report helpful? Give feedback by reacting with 👍 or 👎
No description provided.