twisted / twistedchecker

twistedchecker is a tool to automatically verify code against the Twisted coding standard.
MIT License
9 stars 13 forks source link

Test method names missing underscores aren't detected #62

Closed ghost closed 8 years ago

ghost commented 10 years ago

Test methods named "testFooBar" aren't flagged as faulty. They should be named test_FooBar.

I'm not sure what the appropriate way to implement this is. Presumably:

  1. In any module trial would flag as being a test module
  2. Methods with names starting with "test" must also start with "test_"

Imported from Launchpad using lp2gh.

ghost commented 10 years ago

(by lvh) I came across this here:

http://buildbot.twistedmatrix.com/builders/twistedchecker/builds/1453/steps/run-twistedchecker/logs/new%20twistedchecker%20errors

But that has test methods like:

https://twistedmatrix.com/trac/browser/branches/logfilepy3k-6749/twisted/test/test_logfile.py?rev=40475#L43

Which clearly should be flagged :)

adiroiban commented 9 years ago

I am trying to work on this but I am confused by current good/bad method names examples from the test.

As a start I would like to update good/bad test with examples and then I can work on fixing.

Please check current tests and let me know what needs to be changes.

Thanks!

glyph commented 9 years ago

Looks like this still needs some more work, since the tests are failing - I suspect the breakage is just that the regex needs to be updated? I think the test examples look basically correct…

adiroiban commented 9 years ago

As discussed over IRC the tests are not correct as test_SOME_THING should be a valid.


I have updated the tests to include testFooBar... but the test suite is messed up due to changes in #89

I will work on fixin this in a separate ticket and come back once it is fixed.


In an ideal world we will no longer need the narrative docs for styleguide and just have good tests with good docstrings to document the styleguide.

adiroiban commented 9 years ago

Problem

method names were checked using the same regex for both production and test code.

Changes

I have disabled the pylint building / regex based method name checker.

I have renamed the modulename.py checker into a generic check for naming conventions.

I have updated it with check for methods... with a special condition for names starting with test_ or if a test start with testX

I have merged the functional test into a single file for both valid and invalid test.

It will accept dispatched methods like 'render_XXX' or 'ftp_XXX' based on looking at sibling methods.

I have wrote code for both test and non test methods as there were no existing test for the method names.

How to test

check that the examples from the new functional test are valid.

glyph commented 8 years ago

Trying to keep up with the github "review queue" here, I noticed this one.

It looks pretty good. I am not sure what the delta will be like on the entire Twisted codebase, but, we can always roll it back if we run into an issue.

glyph commented 8 years ago

I think we may want to roll this back.

This buildbot build:

https://buildbot.twistedmatrix.com/builders/twistedchecker/builds/44/steps/run-twistedchecker/logs/new%20twistedchecker%20errors

suggests that it provokes lots of spurious warnings.

@adiroiban - thoughts?