@matheusbsbs / matheus-bagliano / commits / 70e24186c0
Fork 0

t6022, t6046: fix flaky files-are-updated checks

Several tests wanted to verify that files were actually modified by a merge, which it would do by checking that the mtime was updated. In order to avoid problems with the merge completing so fast that the mtime at the beginning and end of the operation was the same, these tests would first set the mtime of a file to something "old". This "old" value was usually determined as current system clock minus one second, truncated to the nearest integer. Unfortunately, it appears the system clock and filesystem clock are different and comparing across the two runs into race problems resulting in flaky tests. From https://stackoverflow.com/questions/14392975/timestamp-accuracy-on-ext4-sub-millsecond: date will call the gettimeofday system call which will always return the most accurate time available based on the cached kernel time, adjusted by the CPU cycle time if available to give nanosecond resolution. The timestamps stored in the file system however, are only based on the cached kernel time. ie The time calculated at the last timer interrupt. and from https://apenwarr.ca/log/20181113: Does mtime get set to >= the current time? No, this depends on clock granularity. For example, gettimeofday() can return times in microseconds on my system, but ext4 rounds timestamps down to the previous ~10ms (but not exactly 10ms) increment, with the surprising result that a newly-created file is almost always created in the past: $ python -c " import os, time t0 = time.time() open('testfile', 'w').close() print os.stat('testfile').st_mtime - t0 " -0.00234484672546 So, instead of trying to compare across what are effectively two different clocks, just avoid using the system clock. Any new updates to files have to give an mtime at least as big as what is already in the file, so we could define "old" as one second before the mtime found in the file before the merge starts. But, to avoid problems with leap seconds, ntp updates, filesystems that only provide two second resolution, and other such weirdness, let's just pick an hour before the mtime found in the file before the merge starts. Also, clarify in one test where we check the mtime of different files that it really was intentional. I totally forgot the reasons for that and assumed it was a bug when asked. Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Mar 13, 2020 at 20:03 UTC 70e24186c08a125a8714df0e3f3ce10fa7b071f1
2 files changed +17 -23
t/t6022-merge-rename.sh
+10 -9
@@ -243,8 +243,7 @@ test_expect_success 'merge of identical changes in a renamed file' '
243 git reset --hard &&
244 git checkout change+rename &&
245
246 - test-tool chmtime =31337 B &&
247 - test-tool chmtime --get B >old-mtime &&
246 + test-tool chmtime --get -3600 B >old-mtime &&
247 GIT_MERGE_VERBOSITY=3 git merge change >out &&
248
249 test-tool chmtime --get B >new-mtime &&
@@ -253,10 +252,12 @@ test_expect_success 'merge of identical changes in a renamed file' '
252 git reset --hard HEAD^ &&
253 git checkout change &&
254
256 - test-tool chmtime =-1 M &&
257 - test-tool chmtime --get M >old-mtime &&
255 + # A will be renamed to B; we check mtimes and file presence
256 + test_path_is_missing B &&
257 + test-tool chmtime --get -3600 A >old-mtime &&
258 GIT_MERGE_VERBOSITY=3 git merge change+rename >out &&
259
260 + test_path_is_missing A &&
261 test-tool chmtime --get B >new-mtime &&
262 test $(cat old-mtime) -lt $(cat new-mtime)
263 '
@@ -645,7 +646,7 @@ test_expect_success 'setup avoid unnecessary update, normal rename' '
646
647 test_expect_success 'avoid unnecessary update, normal rename' '
648 git checkout -q avoid-unnecessary-update-1^0 &&
648 - test-tool chmtime --get =1000000000 rename >expect &&
649 + test-tool chmtime --get -3600 rename >expect &&
650 git merge merge-branch-1 &&
651 test-tool chmtime --get rename >actual &&
652 test_cmp expect actual # "rename" should have stayed intact
@@ -677,7 +678,7 @@ test_expect_success 'setup to test avoiding unnecessary update, with D/F conflic
678
679 test_expect_success 'avoid unnecessary update, with D/F conflict' '
680 git checkout -q avoid-unnecessary-update-2^0 &&
680 - test-tool chmtime --get =1000000000 df >expect &&
681 + test-tool chmtime --get -3600 df >expect &&
682 git merge merge-branch-2 &&
683 test-tool chmtime --get df >actual &&
684 test_cmp expect actual # "df" should have stayed intact
@@ -708,7 +709,7 @@ test_expect_success 'setup avoid unnecessary update, dir->(file,nothing)' '
709
710 test_expect_success 'avoid unnecessary update, dir->(file,nothing)' '
711 git checkout -q master^0 &&
711 - test-tool chmtime --get =1000000000 df >expect &&
712 + test-tool chmtime --get -3600 df >expect &&
713 git merge side &&
714 test-tool chmtime --get df >actual &&
715 test_cmp expect actual # "df" should have stayed intact
@@ -737,7 +738,7 @@ test_expect_success 'setup avoid unnecessary update, modify/delete' '
738
739 test_expect_success 'avoid unnecessary update, modify/delete' '
740 git checkout -q master^0 &&
740 - test-tool chmtime --get =1000000000 file >expect &&
741 + test-tool chmtime --get -3600 file >expect &&
742 test_must_fail git merge side &&
743 test-tool chmtime --get file >actual &&
744 test_cmp expect actual # "file" should have stayed intact
@@ -765,7 +766,7 @@ test_expect_success 'setup avoid unnecessary update, rename/add-dest' '
766
767 test_expect_success 'avoid unnecessary update, rename/add-dest' '
768 git checkout -q master^0 &&
768 - test-tool chmtime --get =1000000000 newfile >expect &&
769 + test-tool chmtime --get -3600 newfile >expect &&
770 git merge side &&
771 test-tool chmtime --get newfile >actual &&
772 test_cmp expect actual # "file" should have stayed intact
t/t6046-merge-skip-unneeded-updates.sh
+7 -14
@@ -71,8 +71,7 @@ test_expect_success '1a-L: Modify(A)/Modify(B), change on B subset of A' '
71
72 git checkout A^0 &&
73
74 - test-tool chmtime =-1 b &&
75 - test-tool chmtime --get b >old-mtime &&
74 + test-tool chmtime --get -3600 b >old-mtime &&
75
76 GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err &&
77
@@ -102,8 +101,7 @@ test_expect_success '1a-R: Modify(A)/Modify(B), change on B subset of A' '
101
102 git checkout B^0 &&
103
105 - test-tool chmtime =-1 b &&
106 - test-tool chmtime --get b >old-mtime &&
104 + test-tool chmtime --get -3600 b >old-mtime &&
105 GIT_MERGE_VERBOSITY=3 git merge -s recursive A^0 >out 2>err &&
106
107 # Make sure b WAS updated
@@ -198,8 +196,7 @@ test_expect_success '2a-R: Modify/rename, merge into rename side' '
196
197 git checkout B^0 &&
198
201 - test-tool chmtime =-1 c &&
202 - test-tool chmtime --get c >old-mtime &&
199 + test-tool chmtime --get -3600 c >old-mtime &&
200 GIT_MERGE_VERBOSITY=3 git merge -s recursive A^0 >out 2>err &&
201
202 # Make sure c WAS updated
@@ -266,8 +263,7 @@ test_expect_success '2b-L: Rename+Mod(A)/Mod(B), B mods subset of A' '
263
264 git checkout A^0 &&
265
269 - test-tool chmtime =-1 c &&
270 - test-tool chmtime --get c >old-mtime &&
266 + test-tool chmtime --get -3600 c >old-mtime &&
267 GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err &&
268
269 test_must_be_empty err &&
@@ -373,8 +369,7 @@ test_expect_success '2c: Modify b & add c VS rename b->c' '
369
370 git checkout A^0 &&
371
376 - test-tool chmtime =-1 c &&
377 - test-tool chmtime --get c >old-mtime &&
372 + test-tool chmtime --get -3600 c >old-mtime &&
373 GIT_MERGE_VERBOSITY=3 &&
374 export GIT_MERGE_VERBOSITY &&
375 test_must_fail git merge -s recursive B^0 >out 2>err &&
@@ -679,8 +674,7 @@ test_expect_failure '4a: Change on A, change on B subset of A, dirty mods presen
674 git checkout A^0 &&
675 echo "File rewritten" >b &&
676
682 - test-tool chmtime =-1 b &&
683 - test-tool chmtime --get b >old-mtime &&
677 + test-tool chmtime --get -3600 b >old-mtime &&
678
679 GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err &&
680
@@ -747,8 +741,7 @@ test_expect_success '4b: Rename+Mod(A)/Mod(B), change on B subset of A, dirty mo
741 git checkout A^0 &&
742 echo "File rewritten" >c &&
743
750 - test-tool chmtime =-1 c &&
751 - test-tool chmtime --get c >old-mtime &&
744 + test-tool chmtime --get -3600 c >old-mtime &&
745
746 GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err &&
747