robot-brain-project icon indicating copy to clipboard operation
robot-brain-project copied to clipboard

tests, CI and pep8

Open markroxor opened this issue 7 years ago • 6 comments

@brohrer I suggest that so as to avoid any breaking changes we make through out developing Becca, we should write tests for each module and use some sort of continuous integration like Travis CI which requires a .travis.yml file something like this.

We can use tox to automate our tests. I also recommend enforcing pep8 test along with tox by using something like flake8.

Raising this issue to discuss these three things.

markroxor avatar Sep 25 '18 16:09 markroxor

@markroxor these sound like solid recommendations for taking the code a step closer to production quality. I agree that we should implement them at some point, but I suggest that we hold off for now. Here is my thinking--please push back where you suspect I'm mistaken.

Writing a full set of tests, automating them, and implementing continuous integration will take a significant amount of effort and maintenance. The benefits will be greater the larger the codebase gets and the larger the user base grows. Our current user base is pretty small :) My sense is that we are not yet on the part of the cost-benefit trade-off to make it worthwhile. Reviewing each others code and encouraging contributors to use flake8 might be a reasonable interim solution.

Another aspect to consider is that the underlying algorithms have been changing significantly with every version, and will probably continue to do so. This makes it even tougher to maintain testing structure.

I don't throw this out to close the conversation, but to encourage it. What are your thoughts?

brohrer avatar Sep 26 '18 01:09 brohrer

I understand your point. It is not required right now but once the repository takes shape we will certainly need it.

I think we should keep this issue open and comeback to it in the future.

Meanwhile how about we use Travis CI just to ensure that pep8 is followed?

markroxor avatar Sep 26 '18 03:09 markroxor

Brilliant idea! I've added you as a collaborator so that you can make and test the necessary changes more easily without me slowing you down. I've enabled Travis CI for all the becca repositories.

BTW, I have absolutely no objection to adding tests, and even automating them with tox, if you feel motivated to do so. I would totally support that. But for now I feel comfortable not committing to 100% test coverage.

brohrer avatar Sep 26 '18 11:09 brohrer

Thank you Brandon. :)

markroxor avatar Sep 26 '18 12:09 markroxor

I have added the travis config and tox with flake8 - the build is passing on my fork. Unfortunately I cant see it active on your Travis. Maybe you have to enable it from there. Please check,

markroxor avatar Sep 26 '18 13:09 markroxor

I think I have it enabled now.

brohrer avatar Oct 01 '18 14:10 brohrer