cogent3 / c3dev

cogent3 developer tools
5 stars 9 forks source link

DEV: add check for test methods #6

Closed GavinHuttley closed 5 years ago

GavinHuttley commented 5 years ago

Original report by GavinH (Bitbucket: 557058:e40c23e1-e273-4527-a2f8-5de5876e870d, ).


Need to add, to included_tests.py, a function for checking whether:

GavinHuttley commented 5 years ago

Original comment by GavinH (Bitbucket: 557058:e40c23e1-e273-4527-a2f8-5de5876e870d, ).


This function will be a new subcommand, ping me if you need an example of what I mean.

GavinHuttley commented 5 years ago

Original comment by T La (Bitbucket: 5c73c2a3cfbc206be909a286, ).


Just want to clarify:

GavinHuttley commented 5 years ago

Original comment by T La (Bitbucket: 5c73c2a3cfbc206be909a286, ).


Actually, rather than having to write a mini python-subset interpreter, it is possible to use dynamic loading to do this also. I see a relatively clean way of implementing the first two checks. Thoughts?

GavinHuttley commented 5 years ago

Original comment by T La (Bitbucket: 5c73c2a3cfbc206be909a286, ).


I have a working prototype script that does roughly these points. Should I upload or keep local?

GavinHuttley commented 5 years ago

Original comment by GavinH (Bitbucket: 557058:e40c23e1-e273-4527-a2f8-5de5876e870d, ).


Implementation to check for "correctly indented"

A valid test method line is indented four spaces, starts with def and method definition references self, e.g.

class SomeTestSet:
    def test_me(self):
        pass
GavinHuttley commented 5 years ago

Original comment by T La (Bitbucket: 5c73c2a3cfbc206be909a286, ).


I'll implement point one. Point 2 is already done! Point 3 is implemented like this: if the method def is not 'setUp' or doesn't start with 'test_', then it is crippled.

GavinHuttley commented 5 years ago

Original comment by T La (Bitbucket: 5c73c2a3cfbc206be909a286, ).


Point one now implemented, I'll upload so we can tweak as required.

GavinHuttley commented 5 years ago

Original comment by T La (Bitbucket: 5c73c2a3cfbc206be909a286, ).


On point one, I originally just enforced that the function indentation had to be more than the class indentation, and it had to be consistent. Not sure if forcing tests to have 4 spaces of indentation will lose generality or not. I know its just a convention, but still... there's always one!

GavinHuttley commented 5 years ago

Original comment by T La (Bitbucket: 5c73c2a3cfbc206be909a286, ).


Just pushed the script into the repo. I ran the script over our test suite and there are lots of “errors” by what I currently have as the checks. I’d like to be more specific so that we don’t get these false positives, but I also want to make sure that I don’t ignore true problems in the suite. My main concern is what “crippled” means. Currently, I define crippled functions as functions that don’t start with “test_” or “setup”. This is obviously too strong, so I’d like some feedback on where I can loosen this definition. Testing only for “est_” as in the example in the task description is too strict, and will miss other cases that really should be deemed crippled, so I want to know what these other cases look like precisely.

GavinHuttley commented 5 years ago

Original comment by T La (Bitbucket: 5c73c2a3cfbc206be909a286, ).


Script updated on the server. Are there any other tweaks to the script that should be made? Else, ticket can be closed.

GavinHuttley commented 5 years ago

Original comment by T La (Bitbucket: 5c73c2a3cfbc206be909a286, ).


script added to repo, 098feba