Skip to content

Commit 70e2418

Browse files
newrengitster
authored andcommitted
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 <[email protected]> Signed-off-by: Elijah Newren <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 65bf820 commit 70e2418

File tree

2 files changed

+17
-23
lines changed

2 files changed

+17
-23
lines changed

t/t6022-merge-rename.sh

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,7 @@ test_expect_success 'merge of identical changes in a renamed file' '
243243
git reset --hard &&
244244
git checkout change+rename &&
245245
246-
test-tool chmtime =31337 B &&
247-
test-tool chmtime --get B >old-mtime &&
246+
test-tool chmtime --get -3600 B >old-mtime &&
248247
GIT_MERGE_VERBOSITY=3 git merge change >out &&
249248
250249
test-tool chmtime --get B >new-mtime &&
@@ -253,10 +252,12 @@ test_expect_success 'merge of identical changes in a renamed file' '
253252
git reset --hard HEAD^ &&
254253
git checkout change &&
255254
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 &&
258258
GIT_MERGE_VERBOSITY=3 git merge change+rename >out &&
259259
260+
test_path_is_missing A &&
260261
test-tool chmtime --get B >new-mtime &&
261262
test $(cat old-mtime) -lt $(cat new-mtime)
262263
'
@@ -645,7 +646,7 @@ test_expect_success 'setup avoid unnecessary update, normal rename' '
645646

646647
test_expect_success 'avoid unnecessary update, normal rename' '
647648
git checkout -q avoid-unnecessary-update-1^0 &&
648-
test-tool chmtime --get =1000000000 rename >expect &&
649+
test-tool chmtime --get -3600 rename >expect &&
649650
git merge merge-branch-1 &&
650651
test-tool chmtime --get rename >actual &&
651652
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
677678

678679
test_expect_success 'avoid unnecessary update, with D/F conflict' '
679680
git checkout -q avoid-unnecessary-update-2^0 &&
680-
test-tool chmtime --get =1000000000 df >expect &&
681+
test-tool chmtime --get -3600 df >expect &&
681682
git merge merge-branch-2 &&
682683
test-tool chmtime --get df >actual &&
683684
test_cmp expect actual # "df" should have stayed intact
@@ -708,7 +709,7 @@ test_expect_success 'setup avoid unnecessary update, dir->(file,nothing)' '
708709

709710
test_expect_success 'avoid unnecessary update, dir->(file,nothing)' '
710711
git checkout -q master^0 &&
711-
test-tool chmtime --get =1000000000 df >expect &&
712+
test-tool chmtime --get -3600 df >expect &&
712713
git merge side &&
713714
test-tool chmtime --get df >actual &&
714715
test_cmp expect actual # "df" should have stayed intact
@@ -737,7 +738,7 @@ test_expect_success 'setup avoid unnecessary update, modify/delete' '
737738

738739
test_expect_success 'avoid unnecessary update, modify/delete' '
739740
git checkout -q master^0 &&
740-
test-tool chmtime --get =1000000000 file >expect &&
741+
test-tool chmtime --get -3600 file >expect &&
741742
test_must_fail git merge side &&
742743
test-tool chmtime --get file >actual &&
743744
test_cmp expect actual # "file" should have stayed intact
@@ -765,7 +766,7 @@ test_expect_success 'setup avoid unnecessary update, rename/add-dest' '
765766

766767
test_expect_success 'avoid unnecessary update, rename/add-dest' '
767768
git checkout -q master^0 &&
768-
test-tool chmtime --get =1000000000 newfile >expect &&
769+
test-tool chmtime --get -3600 newfile >expect &&
769770
git merge side &&
770771
test-tool chmtime --get newfile >actual &&
771772
test_cmp expect actual # "file" should have stayed intact

t/t6046-merge-skip-unneeded-updates.sh

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,7 @@ test_expect_success '1a-L: Modify(A)/Modify(B), change on B subset of A' '
7171
7272
git checkout A^0 &&
7373
74-
test-tool chmtime =-1 b &&
75-
test-tool chmtime --get b >old-mtime &&
74+
test-tool chmtime --get -3600 b >old-mtime &&
7675
7776
GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err &&
7877
@@ -102,8 +101,7 @@ test_expect_success '1a-R: Modify(A)/Modify(B), change on B subset of A' '
102101
103102
git checkout B^0 &&
104103
105-
test-tool chmtime =-1 b &&
106-
test-tool chmtime --get b >old-mtime &&
104+
test-tool chmtime --get -3600 b >old-mtime &&
107105
GIT_MERGE_VERBOSITY=3 git merge -s recursive A^0 >out 2>err &&
108106
109107
# Make sure b WAS updated
@@ -198,8 +196,7 @@ test_expect_success '2a-R: Modify/rename, merge into rename side' '
198196
199197
git checkout B^0 &&
200198
201-
test-tool chmtime =-1 c &&
202-
test-tool chmtime --get c >old-mtime &&
199+
test-tool chmtime --get -3600 c >old-mtime &&
203200
GIT_MERGE_VERBOSITY=3 git merge -s recursive A^0 >out 2>err &&
204201
205202
# Make sure c WAS updated
@@ -266,8 +263,7 @@ test_expect_success '2b-L: Rename+Mod(A)/Mod(B), B mods subset of A' '
266263
267264
git checkout A^0 &&
268265
269-
test-tool chmtime =-1 c &&
270-
test-tool chmtime --get c >old-mtime &&
266+
test-tool chmtime --get -3600 c >old-mtime &&
271267
GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err &&
272268
273269
test_must_be_empty err &&
@@ -373,8 +369,7 @@ test_expect_success '2c: Modify b & add c VS rename b->c' '
373369
374370
git checkout A^0 &&
375371
376-
test-tool chmtime =-1 c &&
377-
test-tool chmtime --get c >old-mtime &&
372+
test-tool chmtime --get -3600 c >old-mtime &&
378373
GIT_MERGE_VERBOSITY=3 &&
379374
export GIT_MERGE_VERBOSITY &&
380375
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
679674
git checkout A^0 &&
680675
echo "File rewritten" >b &&
681676
682-
test-tool chmtime =-1 b &&
683-
test-tool chmtime --get b >old-mtime &&
677+
test-tool chmtime --get -3600 b >old-mtime &&
684678
685679
GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err &&
686680
@@ -747,8 +741,7 @@ test_expect_success '4b: Rename+Mod(A)/Mod(B), change on B subset of A, dirty mo
747741
git checkout A^0 &&
748742
echo "File rewritten" >c &&
749743
750-
test-tool chmtime =-1 c &&
751-
test-tool chmtime --get c >old-mtime &&
744+
test-tool chmtime --get -3600 c >old-mtime &&
752745
753746
GIT_MERGE_VERBOSITY=3 git merge -s recursive B^0 >out 2>err &&
754747

0 commit comments

Comments
 (0)