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
Identify All Tests That Do Not Include Atleast 1 Assertion #494
Comments
Hey there, I'd be interested in giving this a crack, but I'm not entirely confident I can do a great job. The way I see it, I'd get to know the codebase for a bit, add an assertion or two here and there, then toss a PR your way so I can get some feedback before I get in deep with all the tests. If that sounds alright then sign me up! But I'll understand if you'd rather leave this to someone who knows the system a bit better. |
@garroadran thanks for your interest! Essentially, what I'd be looking for is adding atleast one assertion to any existing tests that don't have any. Getting more familiar with Cement would be ideal, however there is a lot of example to reference throughout all of the All tests are PyTest, and are simple assertions. The trick will be figuring out what to assert on. The best thing to do now would be exactly what you said... make a small change and pull-request, and slowly build up from there. The last thing I want is a huge PR for everything all at once... and I certainly don't want you wasting time going too deep down the wrong paths. Let me know whatever questions you have... maybe joining slack would help (recently moved from Gitter so it is pretty empty/quiet currently). |
Oh yikes, so sorry. I thought I'd replied to this already! |
@derks |
Thanks! I've already started working on this, but it might be a few days before I can get back to you with anything concrete. It's difficult to find time during the week :( |
Ok, this is only a starter list, but these are the tests that don't have any explicit assertions or I suspect that a few of these are basically tests of the "I assert that calling this method does not raise an exception" style, although it's hard to be sure about the intent behind the test without an actual assertion there.
|
|
@garroadran does that last PR close this out? |
Sorry, I saw your comment about the PR addressing |
Yeah, sorry, I was hoping to be a bit further ahead by now, but it's been a crazy month with work. Should be able to sink my teeth into it a bit more this coming weekend. |
This is the last batch. Should be done in a week, two at the most.
|
Closing out! This was a huge help... Thank you @garroadran! |
A lot of tests have been added over the years which were added only for coverage but not actually asserting anything valuable for testing. Would be helpful to have someone identify those tests, and possibly add at least one assertion to them.
This is a low-priority, good first issue... Please note if you're interested in this issue that it is only relevant to the
portland
branch (Cement 3) currently (or master once portland is merged).The text was updated successfully, but these errors were encountered: