-
Notifications
You must be signed in to change notification settings - Fork 103
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
2021 tax rate fixes and tests #974
Conversation
- Fixes taxes for under $100,000 - Adds tests to confirm amounts
Codecov Report
@@ Coverage Diff @@
## master #974 +/- ##
==========================================
- Coverage 51.71% 51.29% -0.43%
==========================================
Files 222 222
Lines 10213 10227 +14
Branches 1428 1432 +4
==========================================
- Hits 5282 5246 -36
- Misses 4916 4966 +50
Partials 15 15
Continue to review full report at Codecov.
|
expect(CURRENT_YEAR).toEqual(2021) | ||
}) | ||
it('ordinary taxes for single status should be correct', async () => { | ||
expect(Math.round(computeOrdinaryTax(FilingStatus.S, 5))).toEqual(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be handled a bit better I think, no reason you can't just have a CSV file loaded, loop over it and assert f(x) = y
for each row in the file for example, or even doing a .foreach
just on a JS array loaded above here would be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's also a property based testing approach that could work. You could have the tax table you linked imported directly as a CSV, and then generate arbitrary incomes, and assert for known income (I_0, I_1]
and tax T
, and test income I_0 < x <= I_1
, T_0 < f(x) <= T_1
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated it to pull in a CSV, then a few points in every tax table range.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for fixing this
how did you convert this to csv? |
If I remember correctly, I copied it into VS Code, then converted all the tabs to commas and used some regex searches to remove unnecessary lines. |
See https://www.irs.gov/instructions/i1040gi#idm140354391131776
What kind of change does this PR introduce? (delete not applicable)