@@ -115,6 +115,12 @@ static void display(struct progress *progress, uint64_t n, const char *done)
int show_update = 0;
int last_count_len = counters_sb->len;
+ if (test_check_progress && progress->last_value != -1 &&
+ n < progress->last_value)
+ BUG("progress \"%s\" counts backwards %"PRIuMAX" -> %"PRIuMAX,
+ progress->title, (uintmax_t)progress->last_value,
+ (uintmax_t)n);
+
progress->last_value = n;
if (progress->delay && (!progress_update || --progress->delay))
@@ -153,28 +153,6 @@ EOF
test_cmp expect out
'
-# Progress counter goes backwards, this should not happen in practice.
-test_expect_success 'progress shortens - crazy caller' '
- cat >expect <<-\EOF &&
- Working hard: 10% (100/1000)<CR>
- Working hard: 20% (200/1000)<CR>
- Working hard: 0% (1/1000) <CR>
- Working hard: 100% (1000/1000)<CR>
- Working hard: 100% (1000/1000), done.
- EOF
-
- cat >in <<-\EOF &&
- progress 100
- progress 200
- progress 1
- progress 1000
- EOF
- test-tool progress --total=1000 "Working hard" <in 2>stderr &&
-
- show_cr <stderr >out &&
- test_cmp expect out
-'
-
test_expect_success 'progress display with throughput' '
cat >expect <<-\EOF &&
Working hard: 10<CR>
@@ -324,13 +302,26 @@ test_expect_success 'GIT_TEST_CHECK_PROGRESS catches non-matching total' '
grep "BUG:.*total progress does not match" stderr
'
+test_expect_success 'GIT_TEST_CHECK_PROGRESS catches backwards counting' '
+ cat >in <<-\EOF &&
+ progress 2
+ progress 1
+ EOF
+
+ test_must_fail env GIT_TEST_CHECK_PROGRESS=1 \
+ test-tool progress --total=3 "Working hard" <in 2>stderr &&
+ grep "BUG:.*counts backwards" stderr
+'
+
test_expect_success 'tolerate bogus progress without GIT_TEST_CHECK_PROGRESS' '
cat >expect <<-\EOF &&
+ Working hard: 66% (2/3)<CR>
Working hard: 33% (1/3)<CR>
Working hard: 33% (1/3), done.
EOF
cat >in <<-\EOF &&
+ progress 2
progress 1
EOF
(
We had to fix a buggy progress line recently, where the progress counter counted backwards, see 8e118e8490 (pack-objects: update "nr_seen" progress based on pack-reused count, 2021-04-11). Extend GIT_TEST_CHECK_PROGRESS to catch these cases as well, i.e. trigger a BUG() when the counter passed to display_progress() is smaller than the previous value. Note that we allow subsequent display_progress() calls with the same counter value, because: - Strictly speaking, it's not wrong to do so. - Forbidding it might make the code calling display_progress() more complex; I suspect that would be the case with e.g. the "Updating index flags" progress line in 'unpack-trees.c', where the counter is increased in recursive function calls. - We would need to special case the internal display() call in stop_progress_msg(), because it uses the same counter value as the last display_progress() call, which would trigger this BUG(). 't0500-progress-display.sh' countains a few tests that check how shortened progress lines are covered up, and one of them ('progress shortens - crazy caller') shortens the progress line by counting backwards. From now on that test would trigger this BUG(), so remove it; the other test cases cover shortening progress lines sufficiently. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- progress.c | 6 ++++++ t/t0500-progress-display.sh | 35 +++++++++++++---------------------- 2 files changed, 19 insertions(+), 22 deletions(-)