diff mbox series

[3/6] t-reftable-stack: use Git's tempfile API instead of mkstemp()

Message ID 20240806142020.4615-4-chandrapratap3519@gmail.com (mailing list archive)
State Superseded
Headers show
Series t: port reftable/stack_test.c to the unit testing framework | expand

Commit Message

Chandra Pratap Aug. 6, 2024, 2:13 p.m. UTC
Git's tempfile API defined by $GIT_DIR/tempfile.{c, h} provides
a unified interface for tempfile operations. Since reftable/stack.c
uses this API for all its tempfile needs instead of raw functions
like mkstemp(), make the ported stack test strictly use Git's
tempfile API as well.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 t/unit-tests/t-reftable-stack.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Junio C Hamano Aug. 6, 2024, 8:47 p.m. UTC | #1
Chandra Pratap <chandrapratap3519@gmail.com> writes:

> Git's tempfile API defined by $GIT_DIR/tempfile.{c, h} provides

You are using the syntax of (some) shells to express "repeat the
string with 'c' and 'h' appended" here, so drop SP after the comma.
Patrick Steinhardt Aug. 7, 2024, 5:23 a.m. UTC | #2
On Tue, Aug 06, 2024 at 07:43:39PM +0530, Chandra Pratap wrote:
> Git's tempfile API defined by $GIT_DIR/tempfile.{c, h} provides
> a unified interface for tempfile operations. Since reftable/stack.c
> uses this API for all its tempfile needs instead of raw functions
> like mkstemp(), make the ported stack test strictly use Git's
> tempfile API as well.
> 
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
> ---
>  t/unit-tests/t-reftable-stack.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c
> index e033feb8ee..14909b127e 100644
> --- a/t/unit-tests/t-reftable-stack.c
> +++ b/t/unit-tests/t-reftable-stack.c
> @@ -76,7 +76,8 @@ static char *get_tmp_dir(int linenumber)
>  static void t_read_file(void)
>  {
>  	char *fn = get_tmp_template(__LINE__);
> -	int fd = mkstemp(fn);
> +	struct tempfile *tmp = mks_tempfile(fn);
> +	int fd = get_tempfile_fd(tmp);
>  	char out[1024] = "line1\n\nline2\nline3";
>  	int n, err;
>  	char **names = NULL;
> @@ -95,6 +96,7 @@ static void t_read_file(void)
>  		check_str(want[i], names[i]);
>  	free_names(names);
>  	(void) remove(fn);
> +	delete_tempfile(&tmp);
>  }

I'm a bit torn whether this is a good idea because we are using a higher
level interface that doesn't have unit tests itself. As I see it, both
low-level primitives and anything that is already verified via another
set of unit tests can be used when writing unit tests. But as far as I
know, the tempfile interface does not yet have any.

Maybe I'm being overthinking this though.

Ideally, we wouldn't have to care about the underlying issue at all,
namely cleanup of the temporary file. This is something that the clar
brings to us for free: it can create temporary directories that it also
knows to clean up automatically once done.

Patrick
diff mbox series

Patch

diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c
index e033feb8ee..14909b127e 100644
--- a/t/unit-tests/t-reftable-stack.c
+++ b/t/unit-tests/t-reftable-stack.c
@@ -76,7 +76,8 @@  static char *get_tmp_dir(int linenumber)
 static void t_read_file(void)
 {
 	char *fn = get_tmp_template(__LINE__);
-	int fd = mkstemp(fn);
+	struct tempfile *tmp = mks_tempfile(fn);
+	int fd = get_tempfile_fd(tmp);
 	char out[1024] = "line1\n\nline2\nline3";
 	int n, err;
 	char **names = NULL;
@@ -95,6 +96,7 @@  static void t_read_file(void)
 		check_str(want[i], names[i]);
 	free_names(names);
 	(void) remove(fn);
+	delete_tempfile(&tmp);
 }
 
 static int write_test_ref(struct reftable_writer *wr, void *arg)