40thieves/webpack-sentry-plugin

Set up CI

rhys-vdw opened this issue · 5 comments

@schneidmaster's comment:

Part of the problem currently is that the tests locally require/run against a real Sentry instance, which makes it a pain to run because you have to set up a test account, an auth token, etc. I would like to rewrite the tests to use mocks for the external service calls instead, which then makes it much easier to expect full test coverage for new code.

@schneidmaster I think I see what you mean. It will be annoying to run tests in the development environment. Presumably our contributors will already have Sentry accounts - is it asking too much to have them generate a test key?

If we do want to keep the current system (since it'll be less effort) it should be possible to create an accounts and encrypt the private key in travis. I expect Sentry would help us out with a free account for this purpose if necessary, but it looks like you can join up for free.

@rhys-vdw I don't think generating a test key is asking too much, but the tests also create real releases/data and require creating a test project as well, and I personally felt the need to create a separate test account too because sentry keys can't be scoped per-project and I didn't want to risk screwing something up and messing with my company's real data. Not an enormous ask but at least I personally found it to be a pain when contributing.

Either way, I agree a good first step is getting CI running against forked pull requests. CI is already set up on master and on direct repo branches, but it's configured not to run forked PRs, presumably because it's got an API key from 40thieves stored in travis and there's some security risk since anyone could submit a malicious PR that steals your key. A dedicated test account would prevent that risk.

Sorry I've only just got around to this. The reason travis is only set up to run on master is because it doesn't allow you to apply environment variables to "third-party" builds - i.e. those not triggered by me. Or at least I couldn't figure out the config to do so. So the credentials on travis are just for a test account that I created. There's nothing particularly special about it, I just didn't want everyone having access to it's credentials. If anyone knows of a better way of setting it up, please go ahead :)

I've opened #74 which changes the tests to use replayer to record Sentry responses as fixtures, and then the tests use those fixtures by default. This will allow us to resolve this issue as well -- the tests in CI will no longer have to run against a real Sentry server (they will just use the fixtures) so we can delete @40thieves' credentials from Travis and then set it to build all PRs.

Implemented in #74 and #76. I've removed the Sentry credentials from CI and configured it to build all PRs.