Skip to content

Commit e1c5253

Browse files
jherlandgitster
authored andcommitted
t3305: check notes fanout more carefully and robustly
In short, before this patch, this test script: - creates many notes - verifies that all notes in the notes tree has a fanout of 1 - removes most notes - verifies that the notes in the notes tree now has a fanout of 0 The fanout verification only happened twice: after creating all the notes, and after removing most of them. This patch strengthens the test by checking the fanout after _each_ added/removed note: We assert that the switch from fanout 0 -> 1 happens exactly once while adding notes (and that the switch pervades the entire notes tree). Likewise, we assert that the switch from fanout 1 -> 0 happens exactly once while removing notes. Additionally, we decrease the number of notes left after removal, from 50 to 15 notes, in order to ensure that fanout 1 -> 0 transition keeps happening regardless of external factors[1]. [1]: Currently (with the SHA1 hash function and the deterministic object ids of the test environment) the fanout heuristic in the notes code happens to switch from 0 -> 1 at 109 notes, and from 1 -> 0 at 59 notes. However, changing the hash function or other external factors will vary these numbers, and the latter may - in theory - go as low as 15. For more details, please see the discussion at https://public-inbox.org/git/[email protected]/ Cc: Johannes Schindelin <[email protected]> Cc: Brian M. Carlson <[email protected]> Signed-off-by: Johan Herland <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent b6d4d82 commit e1c5253

File tree

1 file changed

+76
-25
lines changed

1 file changed

+76
-25
lines changed

t/t3305-notes-fanout.sh

Lines changed: 76 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,32 @@ test_description='Test that adding/removing many notes triggers automatic fanout
44

55
. ./test-lib.sh
66

7+
path_has_fanout() {
8+
path=$1 &&
9+
fanout=$2 &&
10+
after_last_slash=$((40 - $fanout * 2)) &&
11+
echo $path | grep -q "^\([0-9a-f]\{2\}/\)\{$fanout\}[0-9a-f]\{$after_last_slash\}$"
12+
}
13+
14+
touched_one_note_with_fanout() {
15+
notes_commit=$1 &&
16+
modification=$2 && # 'A' for addition, 'D' for deletion
17+
fanout=$3 &&
18+
diff=$(git diff-tree --no-commit-id --name-status --root -r $notes_commit) &&
19+
path=$(echo $diff | sed -e "s/^$modification[\t ]//") &&
20+
path_has_fanout "$path" $fanout;
21+
}
22+
23+
all_notes_have_fanout() {
24+
notes_commit=$1 &&
25+
fanout=$2 &&
26+
git ls-tree -r --name-only $notes_commit 2>/dev/null |
27+
while read path
28+
do
29+
path_has_fanout $path $fanout || return 1
30+
done
31+
}
32+
733
test_expect_success 'creating many notes with git-notes' '
834
num_notes=300 &&
935
i=0 &&
@@ -20,7 +46,7 @@ test_expect_success 'creating many notes with git-notes' '
2046

2147
test_expect_success 'many notes created correctly with git-notes' '
2248
git log | grep "^ " > output &&
23-
i=300 &&
49+
i=$num_notes &&
2450
while test $i -gt 0
2551
do
2652
echo " commit #$i" &&
@@ -30,34 +56,46 @@ test_expect_success 'many notes created correctly with git-notes' '
3056
test_cmp expect output
3157
'
3258

33-
test_expect_success 'many notes created with git-notes triggers fanout' '
34-
# Expect entire notes tree to have a fanout == 1
35-
git ls-tree -r --name-only refs/notes/commits |
36-
while read path
59+
test_expect_success 'stable fanout 0 is followed by stable fanout 1' '
60+
i=$num_notes &&
61+
fanout=0 &&
62+
while test $i -gt 0
3763
do
38-
echo $path | grep "^../[0-9a-f]*$" || {
39-
echo "Invalid path \"$path\"" &&
40-
return 1;
41-
}
42-
done
64+
i=$(($i - 1)) &&
65+
if touched_one_note_with_fanout refs/notes/commits~$i A $fanout
66+
then
67+
continue
68+
elif test $fanout -eq 0
69+
then
70+
fanout=1 &&
71+
if all_notes_have_fanout refs/notes/commits~$i $fanout
72+
then
73+
echo "Fanout 0 -> 1 at refs/notes/commits~$i" &&
74+
continue
75+
fi
76+
fi &&
77+
echo "Failed fanout=$fanout check at refs/notes/commits~$i" &&
78+
git ls-tree -r --name-only refs/notes/commits~$i &&
79+
return 1
80+
done &&
81+
all_notes_have_fanout refs/notes/commits 1
4382
'
4483

4584
test_expect_success 'deleting most notes with git-notes' '
46-
num_notes=250 &&
85+
remove_notes=285 &&
4786
i=0 &&
4887
git rev-list HEAD |
49-
while test $i -lt $num_notes && read sha1
88+
while test $i -lt $remove_notes && read sha1
5089
do
5190
i=$(($i + 1)) &&
5291
test_tick &&
53-
git notes remove "$sha1" ||
54-
exit 1
92+
git notes remove "$sha1" 2>/dev/null || return 1
5593
done
5694
'
5795

5896
test_expect_success 'most notes deleted correctly with git-notes' '
59-
git log HEAD~250 | grep "^ " > output &&
60-
i=50 &&
97+
git log HEAD~$remove_notes | grep "^ " > output &&
98+
i=$(($num_notes - $remove_notes)) &&
6199
while test $i -gt 0
62100
do
63101
echo " commit #$i" &&
@@ -67,16 +105,29 @@ test_expect_success 'most notes deleted correctly with git-notes' '
67105
test_cmp expect output
68106
'
69107

70-
test_expect_success 'deleting most notes triggers fanout consolidation' '
71-
# Expect entire notes tree to have a fanout == 0
72-
git ls-tree -r --name-only refs/notes/commits |
73-
while read path
108+
test_expect_success 'stable fanout 1 is followed by stable fanout 0' '
109+
i=$remove_notes &&
110+
fanout=1 &&
111+
while test $i -gt 0
74112
do
75-
echo $path | grep -v "^../.*" || {
76-
echo "Invalid path \"$path\"" &&
77-
return 1;
78-
}
79-
done
113+
i=$(($i - 1)) &&
114+
if touched_one_note_with_fanout refs/notes/commits~$i D $fanout
115+
then
116+
continue
117+
elif test $fanout -eq 1
118+
then
119+
fanout=0 &&
120+
if all_notes_have_fanout refs/notes/commits~$i $fanout
121+
then
122+
echo "Fanout 1 -> 0 at refs/notes/commits~$i" &&
123+
continue
124+
fi
125+
fi &&
126+
echo "Failed fanout=$fanout check at refs/notes/commits~$i" &&
127+
git ls-tree -r --name-only refs/notes/commits~$i &&
128+
return 1
129+
done &&
130+
all_notes_have_fanout refs/notes/commits 0
80131
'
81132

82133
test_done

0 commit comments

Comments
 (0)