Open Nahor opened 2 months ago
Oh no! I think those tests were written by me. Can you paste the exact test output of cargo test test_pr
here? (Or on a paste site if it's too large.)
In which timezone do you live? I live in UTC+2, so I would have expected that any error there would have failed immediately on my system, and the tests pass on my machine. So let's figure out what exactly is wrong here :)
You just need to look at the private function file_last_modified_time
. pr
and test_pr
each have their own, but one uses Local
, the other Utc
, which can't be right.
Fixing that one line make the failed tests go down from 18 to 3. The remaining 3 are related to valid_last_modified_template_vars
in test_pr
, which also uses Utc
and also get fixed by switching to Local
.
Note the date in the page numbers, e.g.:
<Apr 27 09:17 2024 new file Page 1
>Apr 27 16:17 2024 new file Page 1
My timezone is UTC-8 and "9:17" is indeed the file local time.
(And just FIY, my initial comment about the format, is not related to my system, pr
actually defaults to yyyy-mm-dd
unless specified otherwise. From pr
info page: ‘--date-format=format’ [...] The default date format is ‘%Y-%m-%d %H:%M’ [...]
)
Oh! That seems to be a much more fundamental issue, and doesn't look related to the code I wrote. Yes, this is a valid issue, but I can't do much about it right now. Everyone else is free to grab this issue :)
Just to be clear, the main issue I was reporting was the wrong timezone (Utc vs Local) and the fact that the test and the tool use different ones.
Then I mentioned the format on the off chance that this could be a format parameter for the (not yet implemented) --date-format
option and setting a proper default. But this was stupid of me since I don't remember ever seeing such a parameter, and I only remember ways of setting the overall local timezone of a process (e.g. TZ environment variable).
I've removed that comment from the initial report to avoid further risk confusion.
I was looking at this issue in a different environment (Docker on windows, using a "VSCode devcontainer"), and I found out why this issue was not reproducible by others: the integration test forcefully sets the timezone to UTC.
$ git grep DEFAULT_ENV
tests/common/util.rs:const DEFAULT_ENV: [(&str, &str); 2] = [("LC_ALL", "C"), ("TZ", "UTC")];
I'm not sure why that didn't affect me, but I suspect it's because there is typically no such env variable on Windows so it's ignored by chrono
in that case.
Changing that DEFAULT_ENV
to point to a different timezone causes all but 8 test_pr
tests to fail unless all Utc
are replaced with Local
in test_pr.rs
Also of note, by default the container is UTC, so running uu_pr
manually is consistent with the tests unless one explicitly sets the timezone (via the TZ
env variable or by running sudo dpkg-reconfigure tzdata
).
On my system (Windows + Msys2), some
pr
integration tests (tests/by-util/test_pr.rs
) fail because they expect UTC times while theuu_pr
usesLocal
. Replacing all mention ofUtc
withLocal
fixes all those tests for me and match the timezone used by Msys2pr
.