Skip to content

Conversation

@1-1sam
Copy link
Contributor

@1-1sam 1-1sam commented Sep 11, 2025

This PR adds a todo test for #21827, which tests that localtime reports the correct value when given too large of a time as argument. This behavior still appears to be broken.


  • This set of changes does not require a perldelta entry.

This commit adds a todo test for GH 21827, which tests that localtime
reports the correct value when given too large of a time as argument.
This behavior still appears to be broken.
@khwilliamson khwilliamson merged commit 3e69d0f into Perl:blead Sep 15, 2025
34 checks passed
@khwilliamson
Copy link
Contributor

@1-1sam this has been merged, but is now showing as passing on my box with both 64 and 32 bit builds

@jkeenan
Copy link
Contributor

jkeenan commented Sep 16, 2025

@1-1sam this has been merged, but is now showing as passing on my box with both 64 and 32 bit builds

@khwilliamson, are you taking into account the fact that the warnings_like test yields 3 different unit tests? As previously reported, on my box, 2 of the 3 are still not passing.

ok 29 - localtime() warning reports correct value when given too large of a number; GH 21827 # TODO GH 21827
not ok 30 - localtime() warning reports correct value when given too large of a number; GH 21827 # TODO GH 21827
not ok 31 - localtime() warning reports correct value when given too large of a number; GH 21827 # TODO GH 21827

@khwilliamson
Copy link
Contributor

@1-1sam I misunderstood the earlier discussion. Our practice is to have no extraneous messages when things are going as expected. That way we don't have to think about if a message means we need to take action or not. The first test that is passing is generating a message that gives us pause every time we see it; it's a distraction. So it needs to go away. The easiest thing I would think is to only have the TODO on the final two messages; that way the first one passing is expected and doesn't generate noise, but should something change and it starts failing, we would know immediately

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