Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
timfish
force-pushed
the
timfish/test/integration-next
branch
3 times, most recently
from
531df3b to
dfa1552
Compare
timfish
force-pushed
the
timfish/test/integration-next
branch
from
dfa1552 to
e93d586
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| if (process.env["SENTRY_TEST_OUT_DIR"]) { | ||
| // eslint-disable-next-line @typescript-eslint/unbound-method | ||
| const { join } = await import("node:path"); | ||
| const { appendFileSync } = await import("node:fs"); | ||
| const path = join(process.env["SENTRY_TEST_OUT_DIR"], "sentry-telemetry.json"); | ||
| appendFileSync(path, JSON.stringify(request) + ",\n"); | ||
| return { statusCode: 200 }; | ||
| } |
There was a problem hiding this comment.
q: Not blocking but would there be a way around this?
There was a problem hiding this comment.
I tried a mock server but spent too long trying to get it working and then found we already did something similar above for the existing integration tests.
I will revisit this though!
| "outputs": [], | ||
| "cache": true | ||
| "inputs": ["default"], | ||
| "cache": false |
There was a problem hiding this comment.
q: That is intentional I guess?
There was a problem hiding this comment.
It was intentional but this probably needs revising too.
I was changing the integration tests and re-running and it was hitting the cache.
Should we ever cache test output?
2 participants
This PR adds a new integration test package which will slowly replace the existing integration tests. For now I have only added Rolldown, partly because it has not been tested up until now. I will slowly migrate the other bundlers over and eventually remove the old integration tests.
The new integration tests run the bundler CLI's which allows us to test both CJS and ESM bundler configuration files. It also allows us to test the bundlers in complete isolation unlike the existing integration tests which call the JavaScript APIs of the bundlers. This has potential to leak context between tests, etc.
Rather than try and read the output files and assert specific code exists in the bundle, in the most part I just snapshot the entire output directory. As long as the snapshots are checked thoroughly for correctness, this is a much more robust way to see when unexpected changes occur. It also lets us test multiple features in less tests.
Worth noting:
@sentry/cli'shelper.jsis patched viapatch-packageso that we can also snapshot the cli args too