okfn-brasil / rosie

🤖 Python application responsible for Serenata de Amor's intelligence
409 stars 60 forks source link

Organize tests of monthly quota and set new limit for vehicle rental #78

Closed rennerocha closed 7 years ago

rennerocha commented 7 years ago

This PR fix #62

I changed how the tests are structured to make it easier for new developers to include new test cases.

Two new fields were included in monthly_subquota_limit_classifier.csv fixture file with the expected test result and a description of the test case to help who is dealing with a failure knows why that test is there.

There still work to be done to review all existing test cases and update all the other test files to start using this structure.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling b1d75c7d8e7d00fc77c87d3527271cfc18f3c7ce on rennerocha:renne-test-organize into on datasciencebr:master.

cuducos commented 7 years ago

There still work to be done to review all existing test cases and update all the other test files to start using this structure.

Should we wait for that or should we move on, review and merge this as an example for further refactors? cc @rennerocha

@anaschwendler — I really like this structure. What do you think?

anaschwendler commented 7 years ago

Hello @rennerocha and @cuducos

@anaschwendler — I really like this structure. What do you think?

Sure! I took a look at this PR a few months ago, but I didn't know how to proceed, because we had change the structure of the project. Can we work in update this PR?

cuducos commented 7 years ago

I took a look at this PR a few months ago, but I didn't know how to proceed, because we had change the structure of the project. Can we work in update this PR?

Those changes are too time consuming and I'm not sure if/when we can afford them. I'd proceed with this PR in spite of the big plans we might never achieve ; )

anaschwendler commented 7 years ago

Hello @rennerocha and @cuducos I removed a few lines in the bottom of the fixture, because I didn't figured out which tests I should address to them.

Soon I'll open a PR for that.