Skip to content
This repository has been archived by the owner on Mar 1, 2018. It is now read-only.

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

Merged
merged 7 commits into from
Nov 30, 2017
Merged

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

merged 7 commits into from
Nov 30, 2017

Conversation

rennerocha
Copy link
Contributor

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
Copy link

Coverage Status

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

@rennerocha rennerocha changed the title Renne test organize Organize tests of monthly quota Sep 4, 2017
@rennerocha rennerocha changed the title Organize tests of monthly quota Organize tests of monthly quota and set new limit for vehicle rental Sep 4, 2017
@cuducos
Copy link
Collaborator

cuducos commented Nov 27, 2017

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
Copy link
Collaborator

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
Copy link
Collaborator

cuducos commented Nov 27, 2017

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 anaschwendler merged commit 7c47e34 into okfn-brasil:master Nov 30, 2017
@anaschwendler
Copy link
Collaborator

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update subquota limit classifier
4 participants