Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit test parameter formatters #708

Merged
merged 10 commits into from Oct 25, 2017
Merged

Conversation

hdoupe
Copy link
Collaborator

@hdoupe hdoupe commented Oct 19, 2017

This PR adds unit tests for functions introduced in PR #641 and related PRs. Tests were added for the following functions:

  1. taxbrain.helpers.get_default_policy_param_name--Two minor bugs were discovered while adding tests and they will be fixed in subsequent commits
  2. taxbrain.helpers.to_json_reform
  3. taxbrain.helpers.parse_errors_warnings
  4. taxbrain.views.read_json_reform

I intended to add tests for the taxbrain.views.get_reform_from_gui and taxbrain.views.get_reform_from_file functions, but this appears to require the mocking of PersonalExemptionForm, TaxSaveInputs, and request objects. I think I'm going to table this for another PR unless someone has an idea for how this could be done more easily.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Oct 19, 2017

This PR is ready for review.

@martinholmer this PR adds tests for most of the parameter mapping functions that were introduced in #641. Would you mind taking a look at the test cases that were used here? It would be excellent if you could think of any edge cases that I missed.

@martinholmer
Copy link
Contributor

@hdoupe said:

PR #708 adds tests for most of the parameter mapping functions that were introduced in #641. Would you mind taking a look at the test cases that were used here? It would be excellent if you could think of any edge cases that I missed.

The number of tests you've added in PR #708 is impressive. This represents a quantum leap in TaxBrain testing. But testing is a process of continuous improvement in which we start with a substantial set of tests (which you definitely have here) and add tests of bugs that are revealed in the future use of TaxBrain.

So, I think what you've done here is great. If bugs are found, you can add a new test that illustrates the bug by failing, and then fix the code so that the new test passes. Does this make sense?

@MattHJensen @GoFroggyRun

@hdoupe
Copy link
Collaborator Author

hdoupe commented Oct 19, 2017

Thanks for the advice @martinholmer. I learned a lot about the value of tests in the last release and plan to do a better job of adding them in the future.

@hdoupe hdoupe merged commit ead6fc4 into ospc-org:master Oct 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants