sentry-python
sentry-python copied to clipboard
Instructions in `CONTRIBUTING.md` lead to an incomplete environment
How do you use Sentry?
Self-hosted/on-premise
Version
1.40.5
Steps to Reproduce
- Clone repo
- Run steps outlined in
CONTRIBUTING.md#Development Environment
Expected Result
pytest tests/ comes up all green
Actual Result
A string of small issues that leads from tests failing to even start to incompatible dependencies version to failures that should not be there.
linter-requirements.txtinstallspymongo, which unmaskspymongointegration tests. They rely onmockupdbbeing installed, but it's only mentioned intox.ini=>pytestblows up on collection. After installingmockupdb, I get 4 errors and 18 warnings (linux aarch64).- Errors are due to
test-requirements.txtnot constraintingpytestversion, triggering bug inpytest-forkedthat is documented intox.ini - Majority of warnings are due to
pytest-asyncionot being installed by anything except fortox(and installing it without version constraints fromtox.inicauses exciting issues)
None of the issues above are serious, but combined they add quite a bit of annoyance to the (ideally) first step of contributing to the project: developer environment setup.
Given the number of additional constraints mentioned in tox.ini, I almost wonder if suggesting using it instead of just pytest might be a good idea (btw tox just worked for me — great job setting it up!)
There is a quick (but brittle) solution, which is to introduce a Yet Another Requirements File for dev env bringup (and maybe make it pull others internally), but if anyone on the team has Thoughts on dependency management management but no time to implement them I'd be happy to lend a hand.
A couple other things related to the test suite that do not warrant separate issues (yet):
-
eventlettests appear to be dead since they're never ran in CI, so explicitly marking them as such (or removing them fromconftest) might be a good call. I spent way too much time wondering why my interpreter is segfaulting and/or hanging while I was unskipping non-integration tests. -
GCP tests are not excluded from
commontest suite, which is a shame since they are by far the slowest part of it:==== slowest 10 durations ==== 15.08s call tests/integrations/gcp/test_gcp.py::test_timeout_error 8.77s call tests/integrations/gcp/test_gcp.py::test_handled_exception 7.17s call tests/integrations/gcp/test_gcp.py::test_error_has_new_trace_context_performance_disabled 7.12s call tests/integrations/gcp/test_gcp.py::test_error_has_existing_trace_context_performance_disabled 6.25s call tests/integrations/gcp/test_gcp.py::test_unhandled_exception 5.13s call tests/integrations/gcp/test_gcp.py::test_traces_sampler_gets_correct_values_in_sampling_context 5.10s call tests/integrations/gcp/test_gcp.py::test_performance_error 5.08s call tests/integrations/gcp/test_gcp.py::test_error_has_new_trace_context_performance_enabled 5.08s call tests/integrations/gcp/test_gcp.py::test_error_has_existing_trace_context_performance_enabled 5.06s call tests/integrations/gcp/test_gcp.py::test_performance_no_error ==== 1016 passed, 143 skipped, 4 warnings in 104.32s (0:01:44) ====Both excluding them from
commonand optimising tests themselves (maybe rebuilding the project for each test is not strictly necessary?) should shave off some seconds from several CI jobs. Unless current behaviour is by design, I can take a stab at it.
Hey @arr-ee, thanks for opening this -- this is all very valuable feedback. Setting up the test suite is currently far from ideal to the point where it's sometimes easier to just submit a PR and have the CI run the tests instead. 🙈
Feel free to suggest improvements to the contributing guide -- I agree that suggesting to use tox directly is a good idea since that's the closest you get to a "standardized" environment that sets almost everything up for you.
As for your other two points:
eventlettests not being run: I noticed this before but didn't investigate if that was intentional or just an oversight -- thanks for finding the relevant commit. We might as well remove the tests and removeeventletfrom any fixtures.- Excluding
gcptests fromcommon: also sounds good to me. As for not rebuilding the project for each test, haven't looked into this so not sure if reusing stuff can't have some side effects, but if not, also sounds good.
At the moment the biggest bottleneck is https://github.com/getsentry/sentry-python/issues/2721 though -- so any smaller gains won't be super noticeable. (Not that we shouldn't do them though. :))
This issue should be fixed by #2761