threads / patch / 17389

patchWindows: Fix intermittent failures of t7701

Subject: [PATCH] Windows: Fix intermittent failures of t7701

## tl;dr

7 messages between Jan 27, 2009 and Jan 28, 2009. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Johannes Sixt· Jan 27, 2009, 13:09 UTC · lore

From: Johannes Sixt <j6t@kdbg.org>

The last test case checks whether unpacked objects receive the time stamp of the pack file. Due to different implementations of stat(2) by MSYS and our version in compat/mingw.c, the test fails in about half of the test runs.

Note the following facts:

- The test uses perl's -M operator to compare the time stamps. Since we
  depend on MSYS perl, the result of this operator is based on MSYS's
  implementation of the stat(2) call.

- NTFS on Windows records fractional seconds.

- The MSYS implementation of stat(2) *rounds* fractional seconds to full
  seconds instead of truncating them. This becomes obvious by comparing the
  modification times reported by 'ls --full-time $f' and 'stat $f' for
  various files $f.

- Our implementation of stat(2) in compat/mingw.c *truncates* to full
  seconds.

The consequence of this is that

- add_packed_git() picks up truncated whole second modification times
  from the pack file time stamp, which is then used for the loose objects,
  while the pack file retains its time stamp in fractional seconds;

- but the test case compares the pack file's rounded modification times
  to the loose objects' truncated modification times.

And half of the time the rounded modification time is not the same as its truncated modification time.

The fix is that we replace perl by 'test-chmtime -v +0', which prints the truncated whole-second mtime without modifying it.

We want to catch failures of test-chmtime; but since it appears in a pipe, we cannot access its exit code. Therefore, we at least make sure that it prints time stamps of all files that are passed on its command line.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/t7701-repack-unpack-unreachable.sh |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

Show changes to t/t7701-repack-unpack-unreachable.sh +9 −6

diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh
index 63a8225..e6e9f99 100755
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -50,12 +50,15 @@ test_expect_success '-A with -d option leaves unreachable objects unpacked' '

 compare_mtimes ()
 {
-	perl -e 'my $reference = shift;
-		 foreach my $file (@ARGV) {
-			exit(1) unless(-f $file && -M $file == -M $reference);
-		 }
-		 exit(0);
-		' -- "$@"
+	test-chmtime -v +0 "$@" |
+	{
+		read ref files &&
+		while read t name; do
+			test $ref = $t || break
+			files="$files $name"
+		done &&
+		test "$files" = "$*"
+	}
 }

 test_expect_success '-A without -d option leaves unreachable objects packed' '

-- 
1.6.1.1.1203.g5882

Johannes Schindelin· Jan 27, 2009, 16:42 UTC · re: Johannes Sixt · lore

Re: [PATCH] Windows: Fix intermittent failures of t7701

Hi,

On Tue, 27 Jan 2009, Johannes Sixt wrote:

> We want to catch failures of test-chmtime; but since it appears in a 
> pipe, we cannot access its exit code. Therefore, we at least make sure 
> that it prints time stamps of all files that are passed on its command 
> line.

I use this trick in my valgrind series:

	($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&
	test 0 = "$(cat exit.code)"

Ciao, Dscho

Jeff King· Jan 28, 2009, 04:28 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Windows: Fix intermittent failures of t7701

On Tue, Jan 27, 2009 at 05:42:03PM +0100, Johannes Schindelin wrote:

Show 9 quoted lines

> > We want to catch failures of test-chmtime; but since it appears in a 
> > pipe, we cannot access its exit code. Therefore, we at least make sure 
> > that it prints time stamps of all files that are passed on its command 
> > line.
> 
> I use this trick in my valgrind series:
> 
> 	($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&
> 	test 0 = "$(cat exit.code)"

Oh, that's far too readable. How about:

  exec 3>&1
  status=$( ( ($PROGRAM ; echo $? >&4) | $OTHER_PROGRAM >&3) 4>&1 )
  exec 3>&-

But seriously, I think if we are talking about tests, then

  $PROGRAM >output &&
  $OTHER_PROGRAM <output

is very clear to read, and as a bonus makes "output" accessible for viewing when the test breaks. The downside is that it isn't very efficient for a large output, but most of our test output is small (and we don't care that much about efficiency).

Just my shell bikeshedding 2 cents. Feel free to ignore.

-Peff

Johannes Schindelin· Jan 28, 2009, 12:43 UTC · re: Jeff King · lore

Re: [PATCH] Windows: Fix intermittent failures of t7701

Hi,

On Tue, 27 Jan 2009, Jeff King wrote:

Show 25 quoted lines

> On Tue, Jan 27, 2009 at 05:42:03PM +0100, Johannes Schindelin wrote:
> 
> > > We want to catch failures of test-chmtime; but since it appears in a 
> > > pipe, we cannot access its exit code. Therefore, we at least make sure 
> > > that it prints time stamps of all files that are passed on its command 
> > > line.
> > 
> > I use this trick in my valgrind series:
> > 
> > 	($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&
> > 	test 0 = "$(cat exit.code)"
> 
> Oh, that's far too readable. How about:
> 
>   exec 3>&1
>   status=$( ( ($PROGRAM ; echo $? >&4) | $OTHER_PROGRAM >&3) 4>&1 )
>   exec 3>&-
> 
> But seriously, I think if we are talking about tests, then
> 
>   $PROGRAM >output &&
>   $OTHER_PROGRAM <output
> 
> is very clear to read, and as a bonus makes "output" accessible for
> viewing when the test breaks.

The real problem is that in my case, OTHER_PROGRAM=tee.

But you're right, Hannes was talking about tests, where it might even make sense to have a record of what was passed between the two programs.

Ciao, Dscho

Johannes Sixt· Jan 28, 2009, 07:26 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Windows: Fix intermittent failures of t7701

Johannes Schindelin schrieb:

> I use this trick in my valgrind series:
> 
> 	($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&
> 	test 0 = "$(cat exit.code)"

Ah, using a file as temporary storage? Why not simply

	$PROGRAM > data &&
	$OTHER_PROGRAM < data

Hm? ;)

-- Hannes

Junio C Hamano· Jan 28, 2009, 08:26 UTC · re: Johannes Sixt · lore

Re: [PATCH] Windows: Fix intermittent failures of t7701

Johannes Sixt <j.sixt@viscovery.net> writes:

Show 12 quoted lines

> Johannes Schindelin schrieb:
>> I use this trick in my valgrind series:
>> 
>> 	($PROGRAM; echo $? > exit.code) | $OTHER_PROGRAM &&
>> 	test 0 = "$(cat exit.code)"
>
> Ah, using a file as temporary storage? Why not simply
>
> 	$PROGRAM > data &&
> 	$OTHER_PROGRAM < data
>
> Hm? ;)

Because too much cleverness often blinds clever minds.

Care to reroll with the simpler, dumber but clearer version?

Johannes Sixt· Jan 28, 2009, 09:52 UTC · re: Junio C Hamano · lore

[PATCH v2] Windows: Fix intermittent failures of t7701

From: Johannes Sixt <j6t@kdbg.org>

Note the following facts:

- The test uses perl's -M operator to compare the time stamps. Since we
  depend on MSYS perl, the result of this operator is based on MSYS's
  implementation of the stat(2) call.

- NTFS on Windows records fractional seconds.

- The MSYS implementation of stat(2) *rounds* fractional seconds to full
  seconds instead of truncating them. This becomes obvious by comparing the
  modification times reported by 'ls --full-time $f' and 'stat $f' for
  various files $f.

- Our implementation of stat(2) in compat/mingw.c *truncates* to full
  seconds.

The consequence of this is that

- add_packed_git() picks up a truncated whole second modification time
  from the pack file time stamp, which is then used for the loose objects,
  while the pack file retains its time stamp in fractional seconds;

- but the test case compared the pack file's rounded modification times
  to the loose objects' truncated modification times.

And half of the time the rounded modification time is not the same as its truncated modification time.

The fix is that we replace perl by 'test-chmtime -v +0', which prints the truncated whole-second mtime without modifying it.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Junio C Hamano schrieb:

> Care to reroll with the simpler, dumber but clearer version?

OK. This time I use an intermediate file and dquotes around $t.

-- Hannes

 t/t7701-repack-unpack-unreachable.sh |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

Show changes to t/t7701-repack-unpack-unreachable.sh +7 −7

diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh
index 63a8225..5babdf2 100755
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -50,12 +50,10 @@ test_expect_success '-A with -d option leaves unreachable objects unpacked' '

 compare_mtimes ()
 {
-	perl -e 'my $reference = shift;
-		 foreach my $file (@ARGV) {
-			exit(1) unless(-f $file && -M $file == -M $reference);
-		 }
-		 exit(0);
-		' -- "$@"
+	read tref rest &&
+	while read t rest; do
+		test "$tref" = "$t" || break
+	done
 }

 test_expect_success '-A without -d option leaves unreachable objects packed' '
@@ -87,7 +85,9 @@ test_expect_success 'unpacked objects receive timestamp of pack file' '
 	tmppack=".git/objects/pack/tmp_pack" &&
 	ln "$packfile" "$tmppack" &&
 	git repack -A -l -d &&
-	compare_mtimes "$tmppack" "$fsha1path" "$csha1path" "$tsha1path"
+	test-chmtime -v +0 "$tmppack" "$fsha1path" "$csha1path" "$tsha1path" \
+		> mtimes &&
+	compare_mtimes < mtimes
 '

 test_done

-- 
1.6.1.1.1203.g5882