test: Remove confusing and failing system time test #32212

1. 
   maflcko commented at 9:18 am on April 3, 2025: member

   This was just added as a sanity check in fa013664ae23d0682a195b9bded85bc19c99536e by myself.

   However, the test uses system time, so it may obviously (albeit rarely) fail.

   Fix it by removing it.

   Can be tested by running two bash loops at the same time:

   while ( ./bld-cmake/bin/test_bitcoin -t util_tests/util_time_GetTime ) ; do true ; done

   while ( date -s "$(date -d 'now + 0.015 seconds' '+%Y-%m-%d %H:%M:%S.%3N')" && date -s "$(date -d 'now - 0.015 seconds' '+%Y-%m-%d %H:%M:%S.%3N')" ) ; do true ; done

   Eventually, it will fail:

   0test/util_tests.cpp(595): error: in "util_tests/util_time_GetTime": check ms_0 < GetTime<std::chrono::milliseconds>() has failed
   1test/util_tests.cpp(596): error: in "util_tests/util_time_GetTime": check us_0 < GetTime<std::chrono::microseconds>() has failed
   2
   3*** 2 failures are detected in the test module "Bitcoin Core Test Suite"
   

2. test: Remove confusing and failing system time test fadf8f078e
3. 
   DrahtBot commented at 9:19 am on April 3, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32212.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	janb84, mabu44, hebasto

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. DrahtBot added the label Tests on Apr 3, 2025
5. 
   janb84 commented at 6:42 pm on April 3, 2025: contributor

   I understand why the test of the imperfect system time was removed. However, now that we’re aware of these imperfections, I’m curious if they might have any downstream effects on the benchmarking process. Should we consider adding a warning about the precision, or am I drawing the wrong conclusions about the found error?"
6. 
   maflcko commented at 6:19 am on April 4, 2025: member

   I’m curious if they might have any downstream effects on the benchmarking process

   I don’t think (and I hope) that system time is not used while benchmarking. Also, it is not the system time that is wrong, it is the test. It is expected for the system time to be adjustable. For example, if the users adjusts the time, or if the system itself decides to adjust it after a clock drift.

   Let me know if there are any other questions about the code changes, or the pull description, and if it can be improved.

7. 
   janb84 commented at 7:02 am on April 4, 2025: contributor

   ACK fadf8f0

   Before change the test fails with the provided loop after it does not (there is no test code any more).

   • Code review ✅
   • Asked question about implications ✅ (thanks for elaborate answer !)
   • Tests ✅
8. 
   mabu44 commented at 5:09 pm on April 6, 2025: none

   Tested ACK fadf8f078e80fcc3a08f7f6f3291b43ae2bafeea Used the testing procedure provided by the author in #32212#issue-2968872932
9. hebasto approved
10. 
    hebasto commented at 10:46 am on April 7, 2025: member

    ACK fadf8f078e80fcc3a08f7f6f3291b43ae2bafeea, tested on Ubuntu 24.10.
11. hebasto merged this on Apr 7, 2025
12. hebasto closed this on Apr 7, 2025

    ---

13. maflcko deleted the branch on Apr 7, 2025

Contributors
maflcko DrahtBot janb84 mabu44 hebasto

Labels
Tests