-
Notifications
You must be signed in to change notification settings - Fork 11
feat: using pytest as a test runner, diversify the tests into unit, integration, and performance tests #1028
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
Conversation
…ntegration, and performance tests
I'm 100% on board with splitting the tests in these 3 categories. Even if you want to always run all categories, at least you can easily control the order in which they do. Unit tests are the bare minimum so they are always a pre-condition. Integration tests may run slower but only run after unit tests make sure you didn't break anything. And there's no point in running performance tests if you don't know yet your changes are even correct. So having these 3, and in this order, makes perfect sense. That said, why split them by category in different directories if they are then marked with the same categories? Not against it, just wondering.
I like But if there's no good or bad (i.e. no regression checks) then
The main complexity I've always faced when trying to add this to my projects is that if the benchmark is too small or too unrealistic, I fear that the latency might fall within the error margin and be subject to too much variability. I know this is a template for a package but unless I'm testing compute-bound functions, the benchmark should probably consider integration tests in a realistic scenario (i.e. warmed up DBs with enough data and plans executed). I'm always wary of automating this part because, when it comes to performance, I'd love to gather as many data points (and plot them!) as possible before drawing any conclusions. Another idea I've been entertaining (and this might be off-topic for the scope of this PR and repo) is counting the number of queries that a function or endpoint executes. That number alone might not tell the full story but in case there's an unexpected latency spike, it can help understand why. For example, a function that loops over something and accidentally does thousand small queries instead of one big join or IN clause or looping over an ORM object with lazy relationships, which ends up with one query per row. |
I’m curious: for test-driven development, would you write primarily unit tests or would you consider writing some integration tests as well?
Agreed, good point!
If I didn’t and I wanted to find e.g. performance tests then I might have to rummage through a bunch of files to find those. I thought it’s just a helpful way of organizing the test files; and because I can reuse the same filenames I thought that’ll help with orienting myself in the test code 🤔 Now if pytest would allow me to mark folders that’d be useful… maybe?
Fair point, agreed! And like we discussed in issue #563 having regression tracking would be great!
I very much agree, and I’ve looked at these performance/regression tests as close siblings of the unit tests, not so much of the overall integration tests. And personally, I’d be more interested in the performance of this or that critical/relevant function… not all of them, anyway.
This sounds like something a (sampling) profiler can do, or some targeted instrumentation? |
I don't want to give the go-to answer but... it depends 🤓 If it's a something that heavily depends on other components, it might be a good idea to add integration tests as part of a TDD loop. That said, we both know that sometimes is difficult to get people to stick to a rigid definition of unit in testing. For years I've worked in projects where the unit was clearly defined and enclosed. From there, we'd start building the test pyramid all the way to the top (i.e. end-to-end integration tests). However, it's difficult and, specially, exhausting to make every single function a testable unit when folks break the code in smaller functions just for the sake of having... well, smaller functions1 🤷
I love a good profiling as much as anyone but profilers tend to be quite verbose. They are a great tool for when you don't know what you're looking for or for when you are just debugging a performance issue (emphasis on debugging). For regression testing, I'd lean more towards targeted instrumentation to extract specific and well-understood metrics that be used to quickly compare runs across versions. For instance, the example I mentioned about tracking the number of DB queries. There's nothing wrong in doing DB queries if they are needed but we know they are expensive by nature so a spike in those may hint an underlying issue. And if that metric is coupled with performance numbers, it can help put the spotlight on a specific part that may need optimization the event of a performance regression. This, of course, requires some previous knowledge of the application as that may not translate well to other applications. Footnotes
|
I usually start by adding a unit test first. For me, a unit test should be fast, require no network calls, and run fully offline. If a unit test can’t meaningfully cover the behavior, then I add an integration test. I agree with @danirivas that unit tests act as a quick precondition check and should catch the obvious issues before we run the slower, more expensive integration tests (which literally cost money). |
5238344 to
ca2e6e5
Compare
Agree. I personally tend to prioritize quick feedback at the risk of having some false positives slip through until the full test-suite runs. If feedback takes 10min, chances are I'll go do something else and probably forget tests were running until the next epoch reminds me to context-switch back to it. |
I have little to add, I very much agree 🤓 |
I have one more thing to add 🙂 I think what I have described does not follow the test-driven approach. I typically write the code first and consider unit tests afterwards, whereas the test-driven approach emphasizes starting by writing a test and then developing your code to pass that test. |
I think we have text-book approaches to engineering, and the real world. Perhaps the best we can do is try to be consistent over time and as a project scales, and also be realistic about why no single one approach will always work. Testing has great value and it bugs me how engineers often neglect to test their own code. [Deleted two hours of opinionated drivel 🤓] |
…it hook, to untie the two pytest invocations
…g the flit command, to ensure we call the venv’s flit and not another pre-installed version
|
The scope for this PR is |
Based on conversations with @behnazh and the How to keep Unit tests and Integrations tests separate in pytest blurb on StackOverflow, this change adds three new Makefile goals:
make test-unit: run the unit tests, just like we have so far;make test-integration: run integration tests; andmake test-performance: run performance tests:The existing goal
make testruns all tests from all three categories.Points to consider: