From patchwork Wed Oct 11 08:27:08 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Siddharth Singh X-Patchwork-Id: 13416870 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 95362CD6115 for ; Wed, 11 Oct 2023 08:27:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344742AbjJKI1b (ORCPT ); Wed, 11 Oct 2023 04:27:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42034 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230177AbjJKI12 (ORCPT ); Wed, 11 Oct 2023 04:27:28 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 40CEA93 for ; Wed, 11 Oct 2023 01:27:26 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id 3f1490d57ef6-d9a5836ab12so2414916276.2 for ; Wed, 11 Oct 2023 01:27:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1697012845; x=1697617645; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:from:subject:message-id :mime-version:date:from:to:cc:subject:date:message-id:reply-to; bh=gDFIJbnVzlh+SNbpSxZrHJZC7jJZ2V/dm7fWCiypn+8=; b=a+JeSS/Xi/JvHsibOyBHZF5R5DyhOaYnLKmDbx7nshnfwc0zMczvr2DG+xU8f3vGwA 6wT7djqxNV8ohVbDXvEnZBs9bfx524cIMCOV6MXBTdPF82oKb2hqSN21XkKe4sRl6mp9 DtFuCIE2lV0biNzAKSwIp+MK9B7EIFz1ckT0cDcaOlZgH7kJMv3dG6T1HjU8erdEjm9j kegNswAHfnTvxugoysCRXNnWB1y8YqHrvEBrXi+hfmnT/wXIO4c/b5vINK/AFnACtnfV WIPVtL3k4OUzeLIOVfsISe6shRfOvxKyATgDyMuLSH3IlQKLqieWPugiEIvkePZ7+2ZD v+/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697012845; x=1697617645; h=content-transfer-encoding:cc:to:from:subject:message-id :mime-version:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=gDFIJbnVzlh+SNbpSxZrHJZC7jJZ2V/dm7fWCiypn+8=; b=QoIwubTCVe2CQKwRTrIp6kUCH3JXWImg8d8czZCCGOCxlH3aw3wkej3laPZDxTchcZ 64DVThfwmxnDop+F0YUxSoAIDLvXEa1+Wu/cQM/Sff/4roONUC6rU9z/x2jXDo5goauT m8nuDyYEr21E+3JhnTVAs7/RD4M69u8qlo4z2MY1gPjis+LvubPsJ67/NgtO0fcW5OKv /qulKCpf9cuUvrNFav6Hj05Kwbty976eBBMH7mYQY6BJIFgjDiJQchTMzR3t8XIhz7Fb UdxDLEBkxIZDzKffeqJctypD/mSO9y4fSEMr7mmRk19Q07J58CQ1PCSOnjV9OD5/qO5Q o/Xg== X-Gm-Message-State: AOJu0YzIKYX/x/giDDJysEQwzm9IOYFIyNJFQ8EBH33iF5ywWpwgcrH6 D8cteWwAAV2ahzYDo6CBqYoElfT1g+fhuMcbJHweoi9cD9GuM53KdisuDm7vm/J4rKakcElzfxA N9k2EX3CrgapLzQuHCir7hk2O09kKlpXqWD5vkc+QDyBJWnDndwUwAhVS5ar1Owejan8+ X-Google-Smtp-Source: AGHT+IEXXJQ3kAwh/VsBFim2u6s45RN4fzqqVVaflQPbja6C8zZpiHAk09WGv/5iHLWwyNMmVFTR1OLQw2awKFmI X-Received: from siddhu.muc.corp.google.com ([2a00:79e0:9c:0:6bc7:72bc:46f9:acd1]) (user=siddhartth job=sendgmr) by 2002:a25:2403:0:b0:d9a:3a14:a5a2 with SMTP id k3-20020a252403000000b00d9a3a14a5a2mr101451ybk.13.1697012845428; Wed, 11 Oct 2023 01:27:25 -0700 (PDT) Date: Wed, 11 Oct 2023 10:27:08 +0200 Mime-Version: 1.0 X-Mailer: git-send-email 2.42.0.609.gbb76f46606-goog Message-ID: <20231011082716.901048-1-siddhartth@google.com> Subject: [PATCH] t-progress.c : unit tests for progress.c From: Siddharth Singh To: git@vger.kernel.org Cc: nasamuffin@google.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org These tests are directly inspired from the tests inside t/t0500-progress-display.sh The existing shell tests for the Git progress library only test the output of the library, not the correctness of the progress struct fields. Unit tests can fill this gap and improve confidence that the library works as expected. For example, unit tests can verify that the progress struct fields are updated correctly when the library is used. Change-Id: I190522f29fdab9291af71b7788eeee2c0f26282d Signed-off-by: Siddharth Singh --- Dear Git Community, As you may be aware, my colleague Josh is proposing a unit testing framework[1] on the mailing list. I attempted to use the latest version of that series to convert t/helper/test-progress.c to unit tests. However, while writing the tests, I realized that the way progress.c is implemented makes it very difficult to test it in units. Firstly, most unit tests are typically written as a method that takes the expected output and the actual output of the unit being tested, and compares them for equality. However, because it's intended to print user-facing output on an interactive terminal, progress.c prints everything out to stderr, which makes it difficult to unit test. As written, a unit test for throughput progress printing would have to capture stdout from the library and compare it to the expected output, which is difficult to implement and unusual for unit tests anyway. There are a few ways to work around this issue in my opinion. One way is to modify the library that does not print to output stream and returns the data to the caller: static void display(struct progress *progress, uint64_t n, char *done){ … } becomes struct strbuf display(struct progress *progress,uint64_t n,char *done){ … } Another way is to capture the output from the library into a file instead of stdout and then read it from the file and compare it to the expected output, However, this is a difficult task, and I recommend against it. We may need to make some changes to the way these libraries are implemented in order to make them unit testable in the future. Therefore, i want to ask if it's worth investing time in developing a solution? I look forward to hearing your thoughts on this. [1] https://lore.kernel.org/git/cover.1692297001.git.steadmon@google.com/ Makefile | 1 + t/unit-tests/.gitignore | 1 + t/unit-tests/t-progress.c | 229 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 231 insertions(+) create mode 100644 t/unit-tests/t-progress.c diff --git a/Makefile b/Makefile index 4016da6e39..eabbfe32cf 100644 --- a/Makefile +++ b/Makefile @@ -1335,6 +1335,7 @@ THIRD_PARTY_SOURCES += sha1dc/% UNIT_TEST_PROGRAMS += t-basic UNIT_TEST_PROGRAMS += t-strbuf +UNIT_TEST_PROGRAMS += t-progress UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_DIR)/%$X,$(UNIT_TEST_PROGRAMS)) UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS)) UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o diff --git a/t/unit-tests/.gitignore b/t/unit-tests/.gitignore index e292d58348..cf5eb4803e 100644 --- a/t/unit-tests/.gitignore +++ b/t/unit-tests/.gitignore @@ -1,2 +1,3 @@ /t-basic /t-strbuf +/t-progress diff --git a/t/unit-tests/t-progress.c b/t/unit-tests/t-progress.c new file mode 100644 index 0000000000..437d6acf16 --- /dev/null +++ b/t/unit-tests/t-progress.c @@ -0,0 +1,229 @@ +#include "test-lib.h" +#include "progress.c" + + +static void t_simple_progress() +{ + int total = 4; + struct progress *progress = NULL; + int i; + progress = start_progress("Working hard", total); + for (i = 1; i <= total; i++) { + display_progress(progress, i); + check_uint(i, ==, progress->last_value); + check_str(progress->title, "Working hard"); + check_int(progress->last_percent, ==, i * 100 / total); + } + return; +} + +static void t_simple_progress_percent_text() +{ + int total = 4; + struct progress *progress = NULL; + int i; + char *expected[] = { + " 0% (0/4)", + " 25% (1/4)", + " 50% (2/4)", + " 75% (3/4)", + "100% (4/4)" + }; + char *instructions[] = { + "progress", + "progress", + "progress", + "progress", + "progress" + }; + int value[] = { + 0, + 1, + 2, + 3, + 4 + }; + progress = start_progress("Working hard", total); + for (i = 0; i < 5; i++) { + if(strcmp(instructions[i], "progress")==0){ + display_progress(progress, value[i]); + check_str(progress->title, "Working hard"); + check_str(progress->counters_sb.buf, expected[i]); + check_uint(i * (100 / total), ==, progress->last_percent); + } + } + return; +} + +static void t_progress_display_breaks_long_lines_1() +{ + int total = 100000; + struct progress *progress = NULL; + int i; + char *expected[4] = { + " 0% (100/100000)", + " 1% (1000/100000)", + " 10% (10000/100000)", + "100% (100000/100000)" + }; + char *instructions[] = { + "progress", + "progress", + "progress", + "progress" + }; + int value[] = { + 100, + 1000, + 10000, + 100000 + }; + progress = start_progress( + "Working hard.......2.........3.........4.........5.........6", + total); + for (i = 0; i < 4; i++) { + if(strcmp(instructions[i], "progress")==0){ + display_progress(progress, value[i]); + } + check_str(progress->title, "Working hard.......2.........3.........4.........5.........6"); + check_str(progress->counters_sb.buf, expected[i]); + } + return; +} + +static void t_progress_display_breaks_long_lines_2() +{ + int total = 100000; + struct progress *progress = NULL; + int i; + char *expected[] = { + "", + " 0% (1/100000)", + "", + " 0% (2/100000)", + " 10% (10000/100000)", + "100% (100000/100000)" + }; + char *instructions[] = { + "update", + "progress", + "update", + "progress", + "progress", + "progress" + }; + int value[] = { + -1, + 1, + -1, + 2, + 10000, + 100000 + }; + progress = start_progress( + "Working hard.......2.........3.........4.........5.........6", + total); + for (i = 0; i < 5; i++) { + if(strcmp(instructions[i], "progress")==0){ + display_progress(progress, value[i]); + check_str(progress->title, "Working hard.......2.........3.........4.........5.........6"); + check_str(progress->counters_sb.buf, expected[i]); + }else if(strcmp(instructions[i], "update")==0){ + progress_test_force_update(); + } + } + return; +} + +static void t_progress_display_breaks_long_lines_3() +{ + int total = 100000; + struct progress *progress = NULL; + int i; + char *expected[4] = { + " 25% (25000/100000)", + " 50% (50000/100000)", + " 75% (75000/100000)", + "100% (100000/100000)" + }; + char *instructions[] = { + "progress", + "progress", + "progress", + "progress" + }; + int value[] = { + 25000, + 50000, + 75000, + 100000 + }; + progress = start_progress( + "Working hard.......2.........3.........4.........5.........6", + total); + for (i = 0; i < 4; i++) { + if(strcmp(instructions[i], "progress")==0){ + display_progress(progress, value[i]); + check_str(progress->title, "Working hard.......2.........3.........4.........5.........6"); + check_str(progress->counters_sb.buf, expected[i]); + }else if(strcmp(instructions[i], "update")==0){ + progress_test_force_update(); + } + } + return; +} + + +static void t_progress_shortens_crazy_caller() +{ + int total = 1000; + struct progress *progress = NULL; + int i; + char *expected[4] = { + " 10% (100/1000)", + " 20% (200/1000)", + " 0% (1/1000)", + "100% (1000/1000)" + }; + char *instructions[] = { + "progress", + "progress", + "progress", + "progress" + }; + int value[] = { + 100, + 200, + 1, + 1000 + }; + progress = start_progress( + "Working hard.......2.........3.........4.........5.........6", + total); + for (i = 0; i < 4; i++) { + if(strcmp(instructions[i], "progress")==0){ + display_progress(progress, value[i]); + check_str(progress->title, "Working hard.......2.........3.........4.........5.........6"); + check_str(progress->counters_sb.buf, expected[i]); + }else if(strcmp(instructions[i], "update")==0){ + progress_test_force_update(); + } + } + return; +} + +int cmd_main(int argc, const char **argv) +{ + TEST(t_simple_progress(), "Simple progress upto 3 units"); + TEST(t_simple_progress_percent_text(), + "Simple progress with percent output"); + TEST(t_progress_display_breaks_long_lines_1(), + "progress display breaks long lines #1"); + TEST(t_progress_display_breaks_long_lines_2(), + "progress display breaks long lines #2"); + TEST(t_progress_display_breaks_long_lines_3(), + "progress display breaks long lines #3"); + TEST(t_progress_shortens_crazy_caller(), + "progress shortens - crazy caller"); + return test_done(); +}