[leap-test] Fix typo in test function name.

[darnuria]

I open this pull-request because I may found a small typo in test cases in leap exercises. I was confused by reading

divisible by 100 not by 3 is still not a leap year

Better change may be:

divisible by 100 but not 400

Please tell me if i am wrong ! :)

[petertseng]

Hmm. So, this is not an easy decision to make. Definitely, the name is confusing. What does 3 have anything to do with leap years? In fact, it doesn't. So why is that test named this way? Let's talk about it and figure out what a good description should be.

For comparison, let's consider one potential implementation of leap

pub fn is_leap_year(year: u64) -> bool {
    let has_factor = |n| year % n == 0;
    has_factor(4) && (!has_factor(100) || has_factor(400))
}

Then we look at exercism/problem-specifications#1581 that says why this case was added.

So, it looks like if we don't have the test for 1900, there is a potential implementation that would pass the tests:

pub fn is_leap_year(year: u64) -> bool {
    let has_factor = |n| year % n == 0;
    has_factor(4) && (!has_factor(3) || has_factor(400))
}

They've replaced 100 with 3, but the tests still pass. Therefore, we need the test for 1900.

But would any student actually submit such an implementation? I don't know. Most students reading the instructions probably would not. A student specifically trying to look for flaws in the tests, maybe.

So is it worth the confusing name? Not sure. Maybe another potential idea to solve this problem is to restructure the tests in some way that doesn't need the confusing name.

[darnuria]

Ok, I understand not expecting something like uncommon factors ahah.

Maybe writing a test with a good panel of years filtering bad factors with a name like: test_uncommon_years_still_not_leap_year ? or just test_still_not_a_leap_year if we want to hide complexity.

Also just writing test_still_not_leap_year_only_divisible_by_4_not_100_unless_by_400.
Thanks for the reply. :)

[coriolinus]

At least for the moment, we're depending on test names derived from the canonical data for identification. (The transition to UUIDs is ongoing.) At least until that transition is complete, we should keep the existing test name.

[darnuria]

Thanks for your response no problem. :)