Update unit tests for new 2024 data and functions#78
Conversation
| # Make sure to remove any years after 2023 from the year vector, since | ||
| # otherwise we risk silently returning null TIF shares for post-2024 TIFs | ||
| year <- year[year <= 2023] |
There was a problem hiding this comment.
This change aligns the behavior of lookup_tif() with its docstring, which states:
Returns 0 rows for any input year after 2023. For tax years 2024 and later, use
lookup_pin_tifto lookup a PIN's TIF share.
There was a problem hiding this comment.
Just to confirm - so this will ensure that even if a user inputs multiple years, such as 2022:2024 the output will include 2022 - 2023 and no rows for 2024?
There was a problem hiding this comment.
That's right! I'm not sure if that's the most intuitive behavior for end users, but it at least conforms with the specification that we describe in the function documentation.
| expect_equal(dim(tax_bill(years[1], pins[1], simplify = TRUE)), c(11, 12)) | ||
| expect_equal(dim(tax_bill(years[1], pins[1], simplify = FALSE)), c(9, 25)) |
There was a problem hiding this comment.
The number of agencies for these PINs has changed in the past few years, so we update the test to reflect the new count of agency line items.
kyrasturgill
left a comment
kyrasturgill
left a comment
There was a problem hiding this comment.
I am not as familiar with unit testing but these additions all make sense to me and functioned when I ran them! I don't really have any edits, just a couple tiny clarifications.
| # Make sure to remove any years after 2023 from the year vector, since | ||
| # otherwise we risk silently returning null TIF shares for post-2024 TIFs | ||
| year <- year[year <= 2023] |
There was a problem hiding this comment.
Just to confirm - so this will ensure that even if a user inputs multiple years, such as 2022:2024 the output will include 2022 - 2023 and no rows for 2024?
There was a problem hiding this comment.
I don't think this actually impacts the output of this test, but should we add the exe_vet_dis_100 here?
…s-to-include-2024-data-and-new-functions
|
@kyrasturgill Do you want to take a final look at this before I merge, or is it good to go? |
jeancochrane
deleted the
jeancochrane/72-update-unit-tests-to-include-2024-data-and-new-functions
branch
2 participants
This PR updates our unit tests so that they test the 2024 changes to data (#63, #65) and functions (#69).
Note that there is one important set of tests that this PR does not touch: The tests that run on sample tax bills stored in the
data-raw/sample_tax_bills/subdirectory. A subsequent PR (#79) will update those tests, since they require the addition of a substantial number of new files, and I want to keep this diff as small as possible. I have also not yet explicitly tested the change to TIF calculation logic intax_bill(), and am planning to do that in #79, since I think it will be easiest to do using sample tax bills.