test tempdir move

Message ID ZwbBsRv2fyVbKWkc@elastic.org
State Committed
Headers
Series test tempdir move |

Commit Message

Frank Ch. Eigler Oct. 9, 2024, 5:47 p.m. UTC
  Hi -

I find the following patch makes make checks bearable again.


From 0acce289fbb9c4da6a6ec1868eed5ede5a62e63d Mon Sep 17 00:00:00 2001
From: "Frank Ch. Eigler" <fche@redhat.com>
Date: Wed, 9 Oct 2024 13:41:14 -0400
Subject: [PATCH] tests/test-subr.sh: Put test_dir under /var/tmp.

Every individual test in elfutils involves a temporary directory.
Previous version of this script put that directory under the build
tree.  That's OK if it's a local disk, but if it's on NFS, then some
tests - run-large-elf-file.sh, several run-debuginfod-*.sh - take long
enough to run to fail tests intermittently.

This patch moves the temp_dir under ${TMPDIR-/var/tmp/}, so it
operates at local disk speed rather than whatever-build-filesystem
speed.  Individual test scripts are all unaffected.  (One could
consider /tmp instead, which is a RAM disk on modern systems, except
that some of the elfutils tests produce GB-sized temporary files.
That's probably too big for RAM.)

Signed-off-by: Frank Ch. Eigler <fche@redhat.com>
---
 tests/test-subr.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Mark Wielaard Oct. 9, 2024, 7:11 p.m. UTC | #1
Hi Frank,

On Wed, Oct 09, 2024 at 01:47:29PM -0400, Frank Ch. Eigler wrote:
> I find the following patch makes make checks bearable again.

I like the idea of this change. Some nitpicks below.

> From 0acce289fbb9c4da6a6ec1868eed5ede5a62e63d Mon Sep 17 00:00:00 2001
> From: "Frank Ch. Eigler" <fche@redhat.com>
> Date: Wed, 9 Oct 2024 13:41:14 -0400
> Subject: [PATCH] tests/test-subr.sh: Put test_dir under /var/tmp.
> 
> Every individual test in elfutils involves a temporary directory.
> Previous version of this script put that directory under the build
> tree.  That's OK if it's a local disk, but if it's on NFS, then some
> tests - run-large-elf-file.sh, several run-debuginfod-*.sh - take long
> enough to run to fail tests intermittently.
> 
> This patch moves the temp_dir under ${TMPDIR-/var/tmp/}, so it
> operates at local disk speed rather than whatever-build-filesystem
> speed.  Individual test scripts are all unaffected.  (One could
> consider /tmp instead, which is a RAM disk on modern systems, except
> that some of the elfutils tests produce GB-sized temporary files.
> That's probably too big for RAM.)

I think ${TMPDIR-/var/tmp/} is the correct choice. /tmp is indeed
supposed to only have small files.

> Signed-off-by: Frank Ch. Eigler <fche@redhat.com>
> ---
>  tests/test-subr.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/test-subr.sh b/tests/test-subr.sh
> index 411e5f288acd..68ef541f8632 100644
> --- a/tests/test-subr.sh
> +++ b/tests/test-subr.sh
> @@ -23,7 +23,7 @@
>  set -e
>  
>  # Each test runs in its own directory to make sure they can run in parallel.
> -test_dir="test-$$"
> +test_dir="${TMPDIR-/var/tmp}/test-$$"

I would make this /test-elfutils-$$ then in case some files are left
around someone can more easily see where they came from.

>  mkdir -p "$test_dir"
>  cd "$test_dir"

There is one "tricky" cleanup issue slightly below this:

# Tests that trap EXIT (0) themselves should call this explicitly.
exit_cleanup()
{
  rm -f $remove_files; cd ..; rmdir $test_dir
}
trap exit_cleanup 0

On clean exit we remove all (tempfiles) and then try to remove the
$test_dir. Note that this fails (the test) if any non-tmp-file is left
behind.

I think this works, but the cd .. doesn't end up where it originally
did. Maybe save the orig_dir="${PWD}" before creating the test_dir and
cd $orig_dir here?

Cheers,

Mark
  
Frank Ch. Eigler Oct. 9, 2024, 8:43 p.m. UTC | #2
Hi -

> I like the idea of this change. Some nitpicks below.

Those were great ideas, v2 below:


From 65efbafc16fffa582a84c277493d2531bf88021a Mon Sep 17 00:00:00 2001
From: "Frank Ch. Eigler" <fche@redhat.com>
Date: Wed, 9 Oct 2024 13:41:14 -0400
Subject: [PATCH] tests/test-subr.sh: Put test_dir under /var/tmp.

Every individual test in elfutils involves a temporary directory.
Previous version of this script put that directory under the build
tree.  That's OK if it's a local disk, but if it's on NFS, then some
tests - run-large-elf-file.sh, several run-debuginfod-*.sh - take long
enough to run to fail tests intermittently.

This patch moves the temp_dir under ${TMPDIR-/var/tmp/}, so it
operates at local disk speed rather than whatever-build-filesystem
speed.  Individual test scripts are all unaffected.  (One could
consider /tmp instead, which is a RAM disk on modern systems, except
that some of the elfutils tests produce GB-sized temporary files.
That's probably too big for RAM.)

Signed-off-by: Frank Ch. Eigler <fche@redhat.com>
---
 tests/test-subr.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/test-subr.sh b/tests/test-subr.sh
index 411e5f288acd..ea80cbec3bc7 100644
--- a/tests/test-subr.sh
+++ b/tests/test-subr.sh
@@ -23,8 +23,9 @@
 set -e
 
 # Each test runs in its own directory to make sure they can run in parallel.
-test_dir="test-$$"
+test_dir="${TMPDIR-/var/tmp}/elfutils-test-$$"
 mkdir -p "$test_dir"
+orig_dir="${PWD}"
 cd "$test_dir"
 
 #LC_ALL=C
@@ -35,7 +36,7 @@ remove_files=
 # Tests that trap EXIT (0) themselves should call this explicitly.
 exit_cleanup()
 {
-  rm -f $remove_files; cd ..; rmdir $test_dir
+  rm -f $remove_files; cd $orig_dir; rmdir $test_dir
 }
 trap exit_cleanup 0
  
Mark Wielaard Oct. 10, 2024, 10:08 a.m. UTC | #3
Hi Frank,

On Wed, Oct 09, 2024 at 04:43:56PM -0400, Frank Ch. Eigler wrote:
> > I like the idea of this change. Some nitpicks below.
> 
> Those were great ideas, v2 below:
> 
> From 65efbafc16fffa582a84c277493d2531bf88021a Mon Sep 17 00:00:00 2001
> From: "Frank Ch. Eigler" <fche@redhat.com>
> Date: Wed, 9 Oct 2024 13:41:14 -0400
> Subject: [PATCH] tests/test-subr.sh: Put test_dir under /var/tmp.
> 
> Every individual test in elfutils involves a temporary directory.
> Previous version of this script put that directory under the build
> tree.  That's OK if it's a local disk, but if it's on NFS, then some
> tests - run-large-elf-file.sh, several run-debuginfod-*.sh - take long
> enough to run to fail tests intermittently.
> 
> This patch moves the temp_dir under ${TMPDIR-/var/tmp/}, so it
> operates at local disk speed rather than whatever-build-filesystem
> speed.  Individual test scripts are all unaffected.  (One could
> consider /tmp instead, which is a RAM disk on modern systems, except
> that some of the elfutils tests produce GB-sized temporary files.
> That's probably too big for RAM.)

Looks good to me.

Thanks,

Mark
  

Patch

diff --git a/tests/test-subr.sh b/tests/test-subr.sh
index 411e5f288acd..68ef541f8632 100644
--- a/tests/test-subr.sh
+++ b/tests/test-subr.sh
@@ -23,7 +23,7 @@ 
 set -e
 
 # Each test runs in its own directory to make sure they can run in parallel.
-test_dir="test-$$"
+test_dir="${TMPDIR-/var/tmp}/test-$$"
 mkdir -p "$test_dir"
 cd "$test_dir"