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

Fix build failure #131

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Conversation

mikenichols
Copy link
Contributor

As of the new year, we started seeing a test failure that I believe was caused by the intersection of:

  1. A test using a relative offset into the past
  2. The need to correctly identify a valid 2-digit number as a prefix for a valid 4-digit year (e.g. 20 is a potentially valid expiration year because you could continue typing and input 2030, which is valid)
  3. The test helper function yearsFromNow which optionally gives you the last 2 digits of a year (e.g. given the current year is 2025, yearsFromNow(-5, 2) will give you "20" as output)

So, why did the test fail?

  • On December 31st, 2024, the code yearsFromNow(-5, 2) returned "19", which is not a potentially valid prefix; the test captured this correctly. However, on January 1st 2025, the code yearsFromNow(-5, 2) returned "20", which is a potentially valid prefix, and the test that was expecting it to not be potentially valid, now fails.

I wanted to make the fix be as unobtrusive as possible, so my preferred solution is to replace the test that was failing with 3 hard-coded tests that show the expected behavior of the year prefixes "19", "20", and "21". This fixes the build with a small diff, and is a robust enough solution to be valid until 2081 when "21" becomes a valid year prefix.

@mikenichols mikenichols requested a review from a team as a code owner January 7, 2025 22:15
@ibooker
Copy link
Contributor

ibooker commented Jan 7, 2025

Does this change warrant a changlog.md entry?

Copy link

@GoogilyBoogily GoogilyBoogily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 🐸

@GoogilyBoogily
Copy link

Does this change warrant a changlog.md entry?

I personally don't think so. I am thinking about it in the same way as version bumps. Source code changes only (excluding tests and docs)

@mikenichols mikenichols merged commit 1fef583 into braintree:main Jan 8, 2025
1 check passed
@mikenichols mikenichols deleted the fix-build-failure branch January 8, 2025 16:48
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.

3 participants