search

opensafely / research-template

The template for new research projects that use the OpenSAFELY framework.
MIT License
16 stars 13 forks source link

Possible test for study definition: patients.satisfying #12

Closed brianmackenna closed 4 years ago

brianmackenna commented 4 years ago

This is an idea for a possible test that would be nice to have but as always to be weighed up with competing priorities. Taking NSAID study def for example, I was able to write the patients.satisfying for the study inclusion/exclusion and include stroke however I had not included and defined stroke anywhere else in the study. It would be nice if there was a test to check for this so ensure we have the right inclusion/exclusion criteria.

Now for studies we are reporting the number of people with a stroke so if was picked up pretty quickly that it was missing but I'm thinking that there may be other studies where this might not be the case and it might be missed.

I've never written tests so I have no idea how much work this would be.

image

evansd commented 4 years ago

@brianmackenna Thanks for flagging this up. I was surprised this didn't throw an error straightaway and then I realised what's happening. It's because the dummy data generation doesn't do any kind of error checking and, because there's no database-url defined, it never loads the real backend code which does the error checking. We'll have to think about how best to manage this, but there shouldn't be any need to write a specific test for this.

sebbacon commented 4 years ago

See linked issue to track progress