[PATCH v3 07/10] t0090: make test pass with SHA-256
One assertion of this test checks for a shrinking cache tree. The initial index contains a cache tree with two directory names but no object ID, and the second index contains a cache tree with an object ID but no directory name. With SHA-1, the second index is smaller than the first, because the directory information stored takes more than the 20 bytes of an SHA-1 hash, but with SHA-256, the hash is longer, and the test fails the assertion that the second index is smaller than the first. To address this issue, increase the length of the subdirectory name to ensure that the cache tree does indeed shrink in size regardless of the algorithm in use. Signed-off-by: brian m. carlson --- t/t0090-cache-tree.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh index 504334e552..ce9a4a5f32 100755 --- a/t/t0090-cache-tree.sh +++ b/t/t0090-cache-tree.sh @@ -162,8 +162,8 @@ test_expect_success PERL 'commit --interactive gives cache-tree on partial commi ' test_expect_success PERL 'commit -p with shrinking cache-tree' ' - mkdir -p deep/subdir && - echo content >deep/subdir/file && + mkdir -p deep/very-long-subdir && + echo content >deep/very-long-subdir/file && git add deep && git commit -m add && git rm -r deep &&
[PATCH v3 05/10] t6030: make test work with SHA-256
Compute several object ID values instead of hard-coding them, and use test_oid_to_path to cleanly produce a path for an object. Note that the bisect code which is tested here remains sensitive to the hash algorithm in use because it uses the object ID to disambiguate between two equidistant commits. Fortunately, SHA-1 and SHA-256 disambiguate identically in the cases we care about, so there is no need to modify the test to accommodate this situation. However, if a further hash algorithm change occurs, this test may require some restructuring. Signed-off-by: brian m. carlson --- t/t6030-bisect-porcelain.sh | 31 --- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 49a394bd75..bdc42e9440 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -615,6 +615,7 @@ test_expect_success 'broken branch creation' ' git add missing/MISSING && git commit -m "6(broken): Added file that will be deleted" && git tag BROKEN_HASH6 && + deleted=$(git rev-parse --verify HEAD:missing) && add_line_into_file "7(broken): second line on a broken branch" hello2 && git tag BROKEN_HASH7 && add_line_into_file "8(broken): third line on a broken branch" hello2 && @@ -622,12 +623,12 @@ test_expect_success 'broken branch creation' ' git rm missing/MISSING && git commit -m "9(broken): Remove missing file" && git tag BROKEN_HASH9 && - rm .git/objects/39/f7e61a724187ab767d2e08442d9b6b9dab587d + rm .git/objects/$(test_oid_to_path $deleted) ' echo "" > expected.ok cat > expected.missing-tree.default < expected.bisect-log < into . -# good: [7b7f204a749c3125d5224ed61ea2ae1187ad046f] Add <2: A new day for git> into . -git bisect start '32a594a3fdac2d57cf6d02987e30eec68511498c' '7b7f204a749c3125d5224ed61ea2ae1187ad046f' -# good: [3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for git> into . -git bisect good 3de952f2416b6084f557ec417709eac740c6818c -# first bad commit: [32a594a3fdac2d57cf6d02987e30eec68511498c] Add <4: Ciao for now> into . +# bad: [$HASH4] Add <4: Ciao for now> into . +# good: [$HASH2] Add <2: A new day for git> into . +git bisect start '$HASH4' '$HASH2' +# good: [$HASH3] Add <3: Another new day for git> into . +git bisect good $HASH3 +# first bad commit: [$HASH4] Add <4: Ciao for now> into . EOF test_expect_success 'bisect log: successful result' ' @@ -731,14 +732,14 @@ test_expect_success 'bisect log: successful result' ' ' cat > expected.bisect-skip-log < into . -# good: [7b7f204a749c3125d5224ed61ea2ae1187ad046f] Add <2: A new day for git> into . -git bisect start '32a594a3fdac2d57cf6d02987e30eec68511498c' '7b7f204a749c3125d5224ed61ea2ae1187ad046f' -# skip: [3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for git> into . -git bisect skip 3de952f2416b6084f557ec417709eac740c6818c +# bad: [$HASH4] Add <4: Ciao for now> into . +# good: [$HASH2] Add <2: A new day for git> into . +git bisect start '$HASH4' '$HASH2' +# skip: [$HASH3] Add <3: Another new day for git> into . +git bisect skip $HASH3 # only skipped commits left to test -# possible first bad commit: [32a594a3fdac2d57cf6d02987e30eec68511498c] Add <4: Ciao for now> into . -# possible first bad commit: [3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for git> into . +# possible first bad commit: [$HASH4] Add <4: Ciao for now> into . +# possible first bad commit: [$HASH3] Add <3: Another new day for git> into . EOF test_expect_success 'bisect log: only skip commits left' '
[PATCH v2 07/10] t0090: make test pass with SHA-256
One assertion of this test checks for a shrinking cache tree. The initial index contains a cache tree with two directory names but no object ID, and the second index contains a cache tree with an object ID but no directory name. With SHA-1, the second index is smaller than the first, because the directory information stored takes more than the 20 bytes of an SHA-1 hash, but with SHA-256, the hash is longer, and the test fails the assertion that the second index is smaller than the first. To address this issue, increase the length of the subdirectory name to ensure that the cache tree does indeed shrink in size regardless of the algorithm in use. Signed-off-by: brian m. carlson --- t/t0090-cache-tree.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh index 504334e552..ce9a4a5f32 100755 --- a/t/t0090-cache-tree.sh +++ b/t/t0090-cache-tree.sh @@ -162,8 +162,8 @@ test_expect_success PERL 'commit --interactive gives cache-tree on partial commi ' test_expect_success PERL 'commit -p with shrinking cache-tree' ' - mkdir -p deep/subdir && - echo content >deep/subdir/file && + mkdir -p deep/very-long-subdir && + echo content >deep/very-long-subdir/file && git add deep && git commit -m add && git rm -r deep &&
[PATCH v2 05/10] t6030: make test work with SHA-256
Compute several object ID values instead of hard-coding them, and use test_oid_to_path to cleanly produce a path for an object. Note that the bisect code which is tested here remains sensitive to the hash algorithm in use because it uses the object ID to disambiguate between two equidistant commits. Fortunately, SHA-1 and SHA-256 disambiguate identically in the cases we care about, so there is no need to modify the test to accommodate this situation. However, if a further hash algorithm change occurs, this test may require some restructuring. Signed-off-by: brian m. carlson --- t/t6030-bisect-porcelain.sh | 31 --- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 49a394bd75..bdc42e9440 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -615,6 +615,7 @@ test_expect_success 'broken branch creation' ' git add missing/MISSING && git commit -m "6(broken): Added file that will be deleted" && git tag BROKEN_HASH6 && + deleted=$(git rev-parse --verify HEAD:missing) && add_line_into_file "7(broken): second line on a broken branch" hello2 && git tag BROKEN_HASH7 && add_line_into_file "8(broken): third line on a broken branch" hello2 && @@ -622,12 +623,12 @@ test_expect_success 'broken branch creation' ' git rm missing/MISSING && git commit -m "9(broken): Remove missing file" && git tag BROKEN_HASH9 && - rm .git/objects/39/f7e61a724187ab767d2e08442d9b6b9dab587d + rm .git/objects/$(test_oid_to_path $deleted) ' echo "" > expected.ok cat > expected.missing-tree.default < expected.bisect-log < into . -# good: [7b7f204a749c3125d5224ed61ea2ae1187ad046f] Add <2: A new day for git> into . -git bisect start '32a594a3fdac2d57cf6d02987e30eec68511498c' '7b7f204a749c3125d5224ed61ea2ae1187ad046f' -# good: [3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for git> into . -git bisect good 3de952f2416b6084f557ec417709eac740c6818c -# first bad commit: [32a594a3fdac2d57cf6d02987e30eec68511498c] Add <4: Ciao for now> into . +# bad: [$HASH4] Add <4: Ciao for now> into . +# good: [$HASH2] Add <2: A new day for git> into . +git bisect start '$HASH4' '$HASH2' +# good: [$HASH3] Add <3: Another new day for git> into . +git bisect good $HASH3 +# first bad commit: [$HASH4] Add <4: Ciao for now> into . EOF test_expect_success 'bisect log: successful result' ' @@ -731,14 +732,14 @@ test_expect_success 'bisect log: successful result' ' ' cat > expected.bisect-skip-log < into . -# good: [7b7f204a749c3125d5224ed61ea2ae1187ad046f] Add <2: A new day for git> into . -git bisect start '32a594a3fdac2d57cf6d02987e30eec68511498c' '7b7f204a749c3125d5224ed61ea2ae1187ad046f' -# skip: [3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for git> into . -git bisect skip 3de952f2416b6084f557ec417709eac740c6818c +# bad: [$HASH4] Add <4: Ciao for now> into . +# good: [$HASH2] Add <2: A new day for git> into . +git bisect start '$HASH4' '$HASH2' +# skip: [$HASH3] Add <3: Another new day for git> into . +git bisect skip $HASH3 # only skipped commits left to test -# possible first bad commit: [32a594a3fdac2d57cf6d02987e30eec68511498c] Add <4: Ciao for now> into . -# possible first bad commit: [3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for git> into . +# possible first bad commit: [$HASH4] Add <4: Ciao for now> into . +# possible first bad commit: [$HASH3] Add <3: Another new day for git> into . EOF test_expect_success 'bisect log: only skip commits left' '
[PATCH 07/10] t0090: make test pass with SHA-256
One assertion of this test checks for a shrinking cache tree. The initial index contains a cache tree with two directory names but no object ID, and the second index contains a cache tree with an object ID but no directory name. With SHA-1, the second index is smaller than the first, because the directory information stored takes more than the 20 bytes of an SHA-1 hash, but with SHA-256, the hash is longer, and the test fails the assertion that the second index is smaller than the first. To address this issue, increase the length of the subdirectory name to ensure that the cache tree does indeed shrink in size regardless of the algorithm in use. Signed-off-by: brian m. carlson --- t/t0090-cache-tree.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh index 504334e552..ce9a4a5f32 100755 --- a/t/t0090-cache-tree.sh +++ b/t/t0090-cache-tree.sh @@ -162,8 +162,8 @@ test_expect_success PERL 'commit --interactive gives cache-tree on partial commi ' test_expect_success PERL 'commit -p with shrinking cache-tree' ' - mkdir -p deep/subdir && - echo content >deep/subdir/file && + mkdir -p deep/very-long-subdir && + echo content >deep/very-long-subdir/file && git add deep && git commit -m add && git rm -r deep &&
[PATCH 05/10] t6030: make test work with SHA-256
Compute several object ID values instead of hard-coding them, and use test_oid_to_path to cleanly produce a path for an object. Note that the bisect code which is tested here remains sensitive to the hash algorithm in use because it uses the object ID to disambiguate between two equidistant commits. Fortunately, SHA-1 and SHA-256 disambiguate identically in the cases we care about, so there is no need to modify the test to accommodate this situation. However, if a further hash algorithm change occurs, this test may require some restructuring. Signed-off-by: brian m. carlson --- t/t6030-bisect-porcelain.sh | 31 --- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 49a394bd75..bdc42e9440 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -615,6 +615,7 @@ test_expect_success 'broken branch creation' ' git add missing/MISSING && git commit -m "6(broken): Added file that will be deleted" && git tag BROKEN_HASH6 && + deleted=$(git rev-parse --verify HEAD:missing) && add_line_into_file "7(broken): second line on a broken branch" hello2 && git tag BROKEN_HASH7 && add_line_into_file "8(broken): third line on a broken branch" hello2 && @@ -622,12 +623,12 @@ test_expect_success 'broken branch creation' ' git rm missing/MISSING && git commit -m "9(broken): Remove missing file" && git tag BROKEN_HASH9 && - rm .git/objects/39/f7e61a724187ab767d2e08442d9b6b9dab587d + rm .git/objects/$(test_oid_to_path $deleted) ' echo "" > expected.ok cat > expected.missing-tree.default < expected.bisect-log < into . -# good: [7b7f204a749c3125d5224ed61ea2ae1187ad046f] Add <2: A new day for git> into . -git bisect start '32a594a3fdac2d57cf6d02987e30eec68511498c' '7b7f204a749c3125d5224ed61ea2ae1187ad046f' -# good: [3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for git> into . -git bisect good 3de952f2416b6084f557ec417709eac740c6818c -# first bad commit: [32a594a3fdac2d57cf6d02987e30eec68511498c] Add <4: Ciao for now> into . +# bad: [$HASH4] Add <4: Ciao for now> into . +# good: [$HASH2] Add <2: A new day for git> into . +git bisect start '$HASH4' '$HASH2' +# good: [$HASH3] Add <3: Another new day for git> into . +git bisect good $HASH3 +# first bad commit: [$HASH4] Add <4: Ciao for now> into . EOF test_expect_success 'bisect log: successful result' ' @@ -731,14 +732,14 @@ test_expect_success 'bisect log: successful result' ' ' cat > expected.bisect-skip-log < into . -# good: [7b7f204a749c3125d5224ed61ea2ae1187ad046f] Add <2: A new day for git> into . -git bisect start '32a594a3fdac2d57cf6d02987e30eec68511498c' '7b7f204a749c3125d5224ed61ea2ae1187ad046f' -# skip: [3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for git> into . -git bisect skip 3de952f2416b6084f557ec417709eac740c6818c +# bad: [$HASH4] Add <4: Ciao for now> into . +# good: [$HASH2] Add <2: A new day for git> into . +git bisect start '$HASH4' '$HASH2' +# skip: [$HASH3] Add <3: Another new day for git> into . +git bisect skip $HASH3 # only skipped commits left to test -# possible first bad commit: [32a594a3fdac2d57cf6d02987e30eec68511498c] Add <4: Ciao for now> into . -# possible first bad commit: [3de952f2416b6084f557ec417709eac740c6818c] Add <3: Another new day for git> into . +# possible first bad commit: [$HASH4] Add <4: Ciao for now> into . +# possible first bad commit: [$HASH3] Add <3: Another new day for git> into . EOF test_expect_success 'bisect log: only skip commits left' '
Re: How to see raw output from 'make test'?
On Thu, Mar 07, 2019 at 12:35:58AM -0500, Jeffrey Walton wrote: > I built Git 2.2.0 from sources. I added CFLAGS="-DNDEBUG -g2 -O1 > -fsantize=undefined'. Then, a 'make test V=1'. The tests don't produce > output. For example: > > *** t0021-conversion.sh *** > ok 1 - setup > [...] Try: make test GIT_TEST_OPTS=-v Or if you have a specific test that's failing, you can just do: make cd t ./t0021-conversion.sh -v -i (the "-i" will stop at the failing test). -Peff
How to see raw output from 'make test'?
Hi Everyone, I built Git 2.2.0 from sources. I added CFLAGS="-DNDEBUG -g2 -O1 -fsantize=undefined'. Then, a 'make test V=1'. The tests don't produce output. For example: *** t0021-conversion.sh *** ok 1 - setup ok 2 - check ok 3 - expanded_in_repo ok 4 - filter shell-escaped filenames ok 5 - required filter should filter data ok 6 - required filter smudge failure ok 7 - required filter clean failure ok 8 - filtering large input to small output should use little memory ok 9 - filter that does not read is fine Yet when I grep for 'runtime error' I am getting some hits for undefined behavior: git-2.21.0$ grep -IR 'runtime error' t/trash directory.t0021-conversion/err:sha1dc/sha1.c:392:2: runtime error: load of misaligned address 0x02d7d1d5 for type 'const uint32_t', which requires 4 byte alignment t/trash directory.t0021-conversion/err:sha1dc/sha1.c:397:2: runtime error: load of misaligned address 0x02d7d1d5 for type 'const uint32_t', which requires 4 byte alignment t/trash directory.t0021-conversion/err:sha1dc/sha1.c:402:2: runtime error: load of misaligned address 0x02d7d1d5 for type 'const uint32_t', which requires 4 byte alignment t/trash directory.t0021-conversion/err:sha1dc/sha1.c:407:2: runtime error: load of misaligned address 0x02d7d1d5 for type 'const uint32_t', which requires 4 byte alignment ... How do I get the full output from 'make test'? Thanks in advance.
Re: avoid "Set preference list" during make test?
On Tue, Aug 28, 2018 at 11:53:55PM +, Tacitus Aedifex wrote: > While running `make test` on the git source tree I keep getting asked: > > Set preference list to: >Cipher: ... >Digest: ... >etc... > > Is there any way to turn that prompt off so that `make test` completes > without any keyboard input? Can you tell us which test is running when you see that? I suspect it might be one of the tests using GnuPG; if so, can you tell us what GnuPG version you're running? Also, just for completeness, which version are you trying to run the tests on, and is it from the repo or a tarball? -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
avoid "Set preference list" during make test?
While running `make test` on the git source tree I keep getting asked: Set preference list to: Cipher: ... Digest: ... etc... Is there any way to turn that prompt off so that `make test` completes without any keyboard input? //tæ
Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Junio C Hamano wrote: > Todd Zullinger writes: >> I wanted to check if this minor patch series had slipped >> under your radar. > > Totally. Queued. > > As they come with Ack by the area maintainer, I'll fast-track them > down to 'master' (other topics typically cook at least for a week in > 'next'). > > Thanks for pinging. Thank you, as always. -- Todd ~~ There is considerable overlap between the intelligence of the smartest bears and the dumbest tourists. -- Park ranger yro.slashdot.org/comments.pl?sid=191810&cid=15757347
Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Todd Zullinger writes: > I wanted to check if this minor patch series had slipped > under your radar. Totally. Queued. As they come with Ack by the area maintainer, I'll fast-track them down to 'master' (other topics typically cook at least for a week in 'next'). Thanks for pinging.
[PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Hi Junio, I wanted to check if this minor patch series had slipped under your radar. If it's waiting patiently in your queue for other topics to advance, that's fine, of course. :) Finished patches: <20171201155653.29553-1-...@pobox.com> and <20171201155653.29553-2-...@pobox.com>. Thanks, -- Todd ~~ Anyone who is capable of getting themselves made President should on no account be allowed to do the job. -- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"
Re: [PATCH v2 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Todd Zullinger wrote: > Reviewed-by: Jonathan Nieder > Signed-off-by: Todd Zullinger > --- > t/lib-git-svn.sh | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) This and the previous one are indeed still Reviewed-by: Jonathan Nieder Thanks.
[PATCH v2 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Setting SVNSERVE_PORT enables several tests which require a local
svnserve daemon to be run (in t9113 & t9126). The tests share setup of
the local svnserve via `start_svnserve()`. The function uses svnserve's
`--listen-once` option, which causes svnserve to accept one connection
on the port, serve it, and exit. When running the tests in parallel
this fails if one test tries to start svnserve while the other is still
running.
Use the test number as the svnserve port (similar to httpd tests) to
avoid port conflicts. Developers can set GIT_TEST_SVNSERVE to any value
other than 'false' or 'auto' to enable these tests.
Acked-by: Eric Wong
Reviewed-by: Jonathan Nieder
Signed-off-by: Todd Zullinger
---
t/lib-git-svn.sh | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 84366b2624..4c1f81f167 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -110,14 +110,16 @@ EOF
}
require_svnserve () {
- if test -z "$SVNSERVE_PORT"
+ test_tristate GIT_TEST_SVNSERVE
+ if ! test "$GIT_TEST_SVNSERVE" = true
then
- skip_all='skipping svnserve test. (set $SVNSERVE_PORT to
enable)'
+ skip_all='skipping svnserve test. (set $GIT_TEST_SVNSERVE to
enable)'
test_done
fi
}
start_svnserve () {
+ SVNSERVE_PORT=${SVNSERVE_PORT-${this_test#t}}
svnserve --listen-port $SVNSERVE_PORT \
--root "$rawsvnrepo" \
--listen-once \
--
2.15.1
Re: [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Jonathan Nieder wrote: Yep, with this description it is Reviewed-by: Jonathan Nieder Thanks for putting up with my nits. :) Thank you for taking the time and looking at the details. :) I'll send a v2 with the changes in the morning, in case there are any other comments (but mostly because it's late and time for a swim). -- Todd ~~ It is impossible to enjoy idling thoroughly unless one has plenty of work to do. -- Jerome K. Jerome
Re: [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Todd Zullinger wrote: > Much better, thank you. How about this for the full commit message: > >t/lib-git-svn.sh: improve svnserve tests with parallel make test > >Setting SVNSERVE_PORT enables several tests which require a local >svnserve daemon to be run (in t9113 & t9126). The tests share setup of >the local svnserve via `start_svnserve()`. The function uses svnserve's >`--listen-once` option, which causes svnserve to accept one connection >on the port, serve it, and exit. When running the tests in parallel >this fails if one test tries to start svnserve while the other is still >running. > >Use the test number as the svnserve port (similar to httpd tests) to >avoid port conflicts. Developers can set GIT_TEST_SVNSERVE to any value >other than 'false' or 'auto' to enable these tests. > > ? Yep, with this description it is Reviewed-by: Jonathan Nieder Thanks for putting up with my nits. :) Jonathan
Re: [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Hi Jonathan, Jonathan Nieder wrote: Todd Zullinger wrote: Previously, setting SVNSERVE_PORT enabled several tests which require a local svnserve daemon to be run (in t9113 & t9126). The tests share the setup of the local svnserve via `start_svnserve()`. The function uses the svnserve option `--listen-once` which causes svnserve to accept one connection on the port, serve it, and exit. When running the tests in parallel this fails if one test tries to start svnserve while the other is still running. I had trouble reading this because I didn't know what previous time it was referring to. Is it about how the option currently behaves? (Git's commit messages tend to use the present tense to describe the behavior before the patch, like a bug report, and the imperative to describe the change the patch proposes to make, like an impolite bug report. :)) This is what I get for skipping grammar classes to go hiking in my youth. But I'm sure I'd do it all again, if given the chance. ;) Use the test number as the svnserve port (similar to httpd tests) to avoid port conflicts. Set GIT_TEST_SVNSERVE to any value other than 'false' or 'auto' to enable these tests. This uses imperative in two ways and also ended up confusing me. The second one is a direction to me, not Git, right? How about: Use the test number instead of $SVNSERVE_PORT as the svnserve port (similar to httpd tests) to avoid port conflicts. Developers can set GIT_TEST_SVNSERVE to any value other than 'false' or 'auto' to enable these tests. Much better, thank you. How about this for the full commit message: t/lib-git-svn.sh: improve svnserve tests with parallel make test Setting SVNSERVE_PORT enables several tests which require a local svnserve daemon to be run (in t9113 & t9126). The tests share setup of the local svnserve via `start_svnserve()`. The function uses svnserve's `--listen-once` option, which causes svnserve to accept one connection on the port, serve it, and exit. When running the tests in parallel this fails if one test tries to start svnserve while the other is still running. Use the test number as the svnserve port (similar to httpd tests) to avoid port conflicts. Developers can set GIT_TEST_SVNSERVE to any value other than 'false' or 'auto' to enable these tests. ? Thanks, -- Todd ~~ Curiosity killed the cat, but for awhile I was a suspect. -- Steven Wright
Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Hi Jonathan, Jonathan Nieder wrote: Todd Zullinger wrote: These tests are not run by default nor are they enabled in travis-ci. I don't know how much testing they get in user or other packager builds. I've been slowly increasing the test suite usage in fedora builds. I ran into this while testing locally with parallel make test. The official fedora builds don't run in parallel (yet), as even before I ran into this issue, builds on the fedora builders randomly failed too often. I'm hoping to eventually enable parallel tests by default though, since it's so much faster. This background could go in the commit message for patch 2, but it also speaks for itself as an obviously good change so I could go either way. Heh. If there's something in there in particular that seems useful, I can certainly add it. I'm not sure what parts of this text would be beneficial to someone down the line though. I usually err on the 'too much information' side of commit messages. I'm happy that it's much harder to do that here. I'd rather have to skim a long message than wonder about the motivation for a change. I'm not sure if there's any objection to changing the variable needed to enable the tests from SVNSERVE_PORT to GIT_TEST_SVNSERVE. The way SVNSERVE_PORT is set in this patch should allow the port to be set explicitly, in case someone requires that -- and they understand that it can fail if running parallel tests, of course. Whether that's a feature or a bug, I'm not sure. :) micronit: can this just say something like Patch 2 is the important one --- see that one for rationale. Patch 1 is an optional preparatory style cleanup. next time? That way, you get an automatic guarantee that all the important information is available in "git log" output to people who need it later. Yeah, I'll try to remember that. I started this without the whitespace cleanup and as I was writing in the single patch description that I didn't think the whitespace cleanup was warranted, I talked myself into doing it as the prep patch. :) Thanks, -- Todd ~~ Tradition: Just because you've always done it that way doesn't mean it's not incredibly stupid. -- Demotivators (www.despair.com)
Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Hi Eric, Eric Wong wrote: I'm fine with this for now. Since svnserve (and git-daemon) both support inetd behavior, I think we can eventually have a test helper which binds random ports and pretends to be an inetd, letting the test run without any special setup. It would let multiple test instances run in parallel, even. Indeed, that would be a nice general improvement. :) Thanks, -- Todd ~~ Suppose I were a member of Congress, and suppose I were an idiot. But, I repeat myself. -- Mark Twain
Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Todd Zullinger wrote: > These tests are not run by default nor are they enabled in travis-ci. I > don't know how much testing they get in user or other packager builds. > > I've been slowly increasing the test suite usage in fedora builds. I > ran into this while testing locally with parallel make test. The > official fedora builds don't run in parallel (yet), as even before I ran > into this issue, builds on the fedora builders randomly failed too > often. I'm hoping to eventually enable parallel tests by default > though, since it's so much faster. This background could go in the commit message for patch 2, but it also speaks for itself as an obviously good change so I could go either way. > I'm not sure if there's any objection to changing the variable needed to > enable the tests from SVNSERVE_PORT to GIT_TEST_SVNSERVE. The way > SVNSERVE_PORT is set in this patch should allow the port to be set > explicitly, in case someone requires that -- and they understand that it > can fail if running parallel tests, of course. Whether that's a > feature or a bug, I'm not sure. :) micronit: can this just say something like Patch 2 is the important one --- see that one for rationale. Patch 1 is an optional preparatory style cleanup. next time? That way, you get an automatic guarantee that all the important information is available in "git log" output to people who need it later. Thanks, Jonathan
Re: [PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Hi,
Todd Zullinger wrote:
> Previously, setting SVNSERVE_PORT enabled several tests which require a
> local svnserve daemon to be run (in t9113 & t9126). The tests share the
> setup of the local svnserve via `start_svnserve()`. The function uses
> the svnserve option `--listen-once` which causes svnserve to accept one
> connection on the port, serve it, and exit. When running the tests in
> parallel this fails if one test tries to start svnserve while the other
> is still running.
I had trouble reading this because I didn't know what previous time it
was referring to. Is it about how the option currently behaves?
(Git's commit messages tend to use the present tense to describe the
behavior before the patch, like a bug report, and the imperative to
describe the change the patch proposes to make, like an impolite bug
report. :))
> Use the test number as the svnserve port (similar to httpd tests) to
> avoid port conflicts. Set GIT_TEST_SVNSERVE to any value other than
> 'false' or 'auto' to enable these tests.
This uses imperative in two ways and also ended up confusing me. The
second one is a direction to me, not Git, right? How about:
Use the test number instead of $SVNSERVE_PORT as the svnserve
port (similar to httpd tests) to avoid port conflicts.
Developers can set GIT_TEST_SVNSERVE to any value other than
'false' or 'auto' to enable these tests.
>
> Signed-off-by: Todd Zullinger
> ---
> t/lib-git-svn.sh | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
The patch looks good. Thanks.
> diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
> index 84366b2624..4c1f81f167 100644
> --- a/t/lib-git-svn.sh
> +++ b/t/lib-git-svn.sh
> @@ -110,14 +110,16 @@ EOF
> }
>
> require_svnserve () {
> - if test -z "$SVNSERVE_PORT"
> + test_tristate GIT_TEST_SVNSERVE
> + if ! test "$GIT_TEST_SVNSERVE" = true
> then
> - skip_all='skipping svnserve test. (set $SVNSERVE_PORT to
> enable)'
> + skip_all='skipping svnserve test. (set $GIT_TEST_SVNSERVE to
> enable)'
> test_done
> fi
> }
>
> start_svnserve () {
> + SVNSERVE_PORT=${SVNSERVE_PORT-${this_test#t}}
> svnserve --listen-port $SVNSERVE_PORT \
>--root "$rawsvnrepo" \
>--listen-once \
> --
> 2.15.1
>
Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Todd Zullinger wrote: > These tests are not run by default nor are they enabled in travis-ci. I > don't know how much testing they get in user or other packager builds. > > I've been slowly increasing the test suite usage in fedora builds. I > ran into this while testing locally with parallel make test. The > official fedora builds don't run in parallel (yet), as even before I ran > into this issue, builds on the fedora builders randomly failed too > often. I'm hoping to eventually enable parallel tests by default > though, since it's so much faster. Cool. > I'm not sure if there's any objection to changing the variable needed to > enable the tests from SVNSERVE_PORT to GIT_TEST_SVNSERVE. The way > SVNSERVE_PORT is set in this patch should allow the port to be set > explicitly, in case someone requires that -- and they understand that it > can fail if running parallel tests, of course. Whether that's a > feature or a bug, I'm not sure. :) I'm fine with this for now. Since svnserve (and git-daemon) both support inetd behavior, I think we can eventually have a test helper which binds random ports and pretends to be an inetd, letting the test run without any special setup. It would let multiple test instances run in parallel, even. > The indentation of lib-git-svn.sh didn't use tabs consistently, in only > a few places, so I cleaned that up first. I can drop that change if > it's unwanted. Fine by me.
[PATCH 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Previously, setting SVNSERVE_PORT enabled several tests which require a
local svnserve daemon to be run (in t9113 & t9126). The tests share the
setup of the local svnserve via `start_svnserve()`. The function uses
the svnserve option `--listen-once` which causes svnserve to accept one
connection on the port, serve it, and exit. When running the tests in
parallel this fails if one test tries to start svnserve while the other
is still running.
Use the test number as the svnserve port (similar to httpd tests) to
avoid port conflicts. Set GIT_TEST_SVNSERVE to any value other than
'false' or 'auto' to enable these tests.
Signed-off-by: Todd Zullinger
---
t/lib-git-svn.sh | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 84366b2624..4c1f81f167 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -110,14 +110,16 @@ EOF
}
require_svnserve () {
- if test -z "$SVNSERVE_PORT"
+ test_tristate GIT_TEST_SVNSERVE
+ if ! test "$GIT_TEST_SVNSERVE" = true
then
- skip_all='skipping svnserve test. (set $SVNSERVE_PORT to
enable)'
+ skip_all='skipping svnserve test. (set $GIT_TEST_SVNSERVE to
enable)'
test_done
fi
}
start_svnserve () {
+ SVNSERVE_PORT=${SVNSERVE_PORT-${this_test#t}}
svnserve --listen-port $SVNSERVE_PORT \
--root "$rawsvnrepo" \
--listen-once \
--
2.15.1
[PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
These tests are not run by default nor are they enabled in travis-ci. I don't know how much testing they get in user or other packager builds. I've been slowly increasing the test suite usage in fedora builds. I ran into this while testing locally with parallel make test. The official fedora builds don't run in parallel (yet), as even before I ran into this issue, builds on the fedora builders randomly failed too often. I'm hoping to eventually enable parallel tests by default though, since it's so much faster. I'm not sure if there's any objection to changing the variable needed to enable the tests from SVNSERVE_PORT to GIT_TEST_SVNSERVE. The way SVNSERVE_PORT is set in this patch should allow the port to be set explicitly, in case someone requires that -- and they understand that it can fail if running parallel tests, of course. Whether that's a feature or a bug, I'm not sure. :) The indentation of lib-git-svn.sh didn't use tabs consistently, in only a few places, so I cleaned that up first. I can drop that change if it's unwanted. Todd Zullinger (2): t/lib-git-svn: whitespace cleanup t/lib-git-svn.sh: improve svnserve tests with parallel make test t/lib-git-svn.sh | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) -- 2.15.1
Re: [PATCH] p3400: make test script executable
René Scharfe writes: > Signed-off-by: Rene Scharfe > --- > This script was added by v2.10.0-rc0~3^2. Thanks. Will merge to 'master'. > > t/perf/p3400-rebase.sh | 0 > 1 file changed, 0 insertions(+), 0 deletions(-) > mode change 100644 => 100755 t/perf/p3400-rebase.sh > > diff --git a/t/perf/p3400-rebase.sh b/t/perf/p3400-rebase.sh > old mode 100644 > new mode 100755
[PATCH] p3400: make test script executable
Signed-off-by: Rene Scharfe --- This script was added by v2.10.0-rc0~3^2. t/perf/p3400-rebase.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 t/perf/p3400-rebase.sh diff --git a/t/perf/p3400-rebase.sh b/t/perf/p3400-rebase.sh old mode 100644 new mode 100755 -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] Make test t3700-add.sh more robust
Ingo Brückl writes: > Subject: Re: [PATCH v2 2/3] Make test t3700-add.sh more robust Please check output from "git shortlog --no-merges -100" to see how your titles play well with others. We typically prefix the title with a specific area, a colon, and a sentence that does not begin in a capital letter and does not end with a full-stop. > Don't rely on chmod to work on the underlying platform (although it > wouldn't harm the result of the '--chmod=-x' test). Directly check the > result of the --chmod option. > > Add a test_mode_in_index helper function in order to check for success. Hmph, I do not immediately see the point of having the helper in test-lib-functions.sh, though. This helper looks more or less specific to this test script. In any case, I think addition of the "test_mode_in_index" helper and conversion from case/esac to the helper should be a separate patch from what this patch wants to do, which is to merge two more-or-less redundant tests into one. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] Make test t3700-add.sh more robust
Don't rely on chmod to work on the underlying platform (although it
wouldn't harm the result of the '--chmod=-x' test). Directly check the
result of the --chmod option.
Add a test_mode_in_index helper function in order to check for success.
Signed-off-by: Ingo Brückl
---
t/t3700-add.sh | 20
t/test-lib-functions.sh | 14 ++
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 494f5b8..c08ec9e 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -332,25 +332,13 @@ test_expect_success 'git add --dry-run --ignore-missing
of non-existing file out
test_i18ncmp expect.err actual.err
'
-test_expect_success 'git add --chmod=+x stages a non-executable file with +x' '
+test_expect_success 'git add --chmod=[+-]x stages correctly' '
rm -f foo1 &&
echo foo >foo1 &&
git add --chmod=+x foo1 &&
- case "$(git ls-files --stage foo1)" in
- 100755" "*foo1) echo pass;;
- *) echo fail; git ls-files --stage foo1; (exit 1);;
- esac
-'
-
-test_expect_success 'git add --chmod=-x stages an executable file with -x' '
- rm -f xfoo1 &&
- echo foo >xfoo1 &&
- chmod 755 xfoo1 &&
- git add --chmod=-x xfoo1 &&
- case "$(git ls-files --stage xfoo1)" in
- 100644" "*xfoo1) echo pass;;
- *) echo fail; git ls-files --stage xfoo1; (exit 1);;
- esac
+ test_mode_in_index 100755 foo1 &&
+ git add --chmod=-x foo1 &&
+ test_mode_in_index 100644 foo1
'
test_expect_success POSIXPERM,SYMLINKS 'git add --chmod=+x with symlinks' '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 4f7eadb..0e6652b 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -990,3 +990,17 @@ test_copy_bytes () {
}
' - "$1"
}
+
+# Test the file mode "$1" of the file "$2" in the index.
+test_mode_in_index () {
+ case "$(git ls-files --stage "$2")" in
+ $1\ *"$2")
+ echo pass
+ ;;
+ *)
+ echo fail
+ git ls-files --stage "$2"
+ return 1
+ ;;
+ esac
+}
--
2.9.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make test Unexpected passes
On Wed, Apr 27, 2016 at 3:05 PM, Junio C Hamano wrote: > Isn't what the test expects bogus in the first place? I'd suggest > removing the test as "pointless waste of resource". > > Comments? > > -- >8 -- Yes, toss it; I find your arguments below compelling. > Manual merge resolution by users fundamentally depends on being able > to tell what is the tracked contents and what is the separator lines > added by "git merge" to tell users which block of lines came from > where. You can deliberately craft a file with lines that resemble > conflict marker lines to make it impossible for the user (here, the > outer merge of merge-recursive also counts as a user of the internal > merge) to tell which part is which, and write a test to demonstrate > that with such a file that "git merge" fundamentally cannot work > with and has to fail on. It however is pointless and waste of time > and resource to run such a test that asserts the obvious. > > In real life, people who do need to track files with such lines that > have as their prefixes set the conflict-marker-size > attribute to make sure they will be able to tell between the tracked > lines that happen to begin with these (confusing) prefixes and the > marker lines that are added by "git merge". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make test Unexpected passes
Elijah Newren writes: > Yeah, the t6036 testcase 'git detects conflict w/ > criss-cross+contrived resolution' could be made to pass by tweaking > the conflict markers. In fact, any tweak would make it appear to > pass, but the test could be updated to still fail by updating the > contrived resolution of a previous merge to use the newer conflict > marker style as well. Isn't what the test expects bogus in the first place? I'd suggest removing the test as "pointless waste of resource". Comments? -- >8 -- Subject: [PATCH] t6036: remove pointless test that expects failure One test in t6036 prepares a file whose contents contain these lines: <<< Temporary merge branch 1 C === B >>> Temporary merge branch 2 and uses recursive merge strategy to run criss-cross merge with it. Manual merge resolution by users fundamentally depends on being able to tell what is the tracked contents and what is the separator lines added by "git merge" to tell users which block of lines came from where. You can deliberately craft a file with lines that resemble conflict marker lines to make it impossible for the user (here, the outer merge of merge-recursive also counts as a user of the internal merge) to tell which part is which, and write a test to demonstrate that with such a file that "git merge" fundamentally cannot work with and has to fail on. It however is pointless and waste of time and resource to run such a test that asserts the obvious. In real life, people who do need to track files with such lines that have as their prefixes set the conflict-marker-size attribute to make sure they will be able to tell between the tracked lines that happen to begin with these (confusing) prefixes and the marker lines that are added by "git merge". Remove the test as pointless waste of resource. Signed-off-by: Junio C Hamano --- t/t6036-recursive-corner-cases.sh | 83 --- 1 file changed, 83 deletions(-) diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index cc1ee6a..c7b0ae6 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -304,89 +304,6 @@ test_expect_success 'git detects conflict merging criss-cross+modify/delete, rev test $(git rev-parse :3:file) = $(git rev-parse B:file) ' -# -# criss-cross + modify/modify with very contrived file contents: -# -# B D -# o---o -# / \ / \ -# A o X ? F -# \ / \ / -# o---o -# C E -# -# Commit A: file with contents 'A\n' -# Commit B: file with contents 'B\n' -# Commit C: file with contents 'C\n' -# Commit D: file with contents 'D\n' -# Commit E: file with contents: -# <<< Temporary merge branch 1 -# C -# === -# B -# >>> Temporary merge branch 2 -# -# Now, when we merge commits D & E, does git detect the conflict? - -test_expect_success 'setup differently handled merges of content conflict' ' - git clean -fdqx && - rm -rf .git && - git init && - - echo A >file && - git add file && - test_tick && - git commit -m A && - - git branch B && - git checkout -b C && - echo C >file && - git add file && - test_tick && - git commit -m C && - - git checkout B && - echo B >file && - git add file && - test_tick && - git commit -m B && - - git checkout B^0 && - test_must_fail git merge C && - echo D >file && - git add file && - test_tick && - git commit -m D && - git tag D && - - git checkout C^0 && - test_must_fail git merge B && - cat>> Temporary merge branch 2 -EOF - git add file && - test_tick && - git commit -m E && - git tag E -' - -test_expect_failure 'git detects conflict w/ criss-cross+contrived resolution' ' - git checkout D^0 && - - test_must_fail git merge -s recursive E^0 && - - test 3 -eq $(git ls-files -s | wc -l) && - test 3 -eq $(git ls-files -u | wc -l) && - test 0 -eq $(git ls-files -o | wc -l) && - - test $(git rev-parse :2:file) = $(git rev-parse D:file) && - test $(git rev-parse :3:file) = $(git rev-parse E:file) -' - # # criss-cross + d/f conflict via add/add: # Commit A: Neither file 'a' nor directory 'a/' exists. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make test Unexpected passes
Ben Woosley writes: > These know breakages: > > ok 50 - rebase -m --onto --root > ok 54 - rebase -m without --onto --root with disjoint history > > Have to do with rebasing a root/orphan branch with the -m flag, > which defaults to -- merge=recursive, which is the case the patch fixed. > > Here are the necessary changes: > ... Thanks, will squash them in and reword the log message accordingly. git-rebase--merge: don't include absent parent as a base Absent this fix, attempts to rebase an orphan branch using "rebase -m" fails with: $ git rebase -m ORPHAN_TARGET_BASE First, rewinding head to replay your work on top of it... ... Note the default rebase behavior does not fail: $ git rebase ORPHAN_TARGET_BASE First, rewinding head to replay your work on top of it... Applying: ORPHAN_ROOT_COMMIT_MSG Using index info to reconstruct a base tree... A few tests were expecting the old behaviour to forbid rebasing such a history with "rebase -m", which now need to expect them to succeed. Signed-off-by: Ben Woosley Signed-off-by: Junio C Hamano -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make test Unexpected passes
Ramsay Jones ramsayjones.plus.com> writes:
>
> Hi Ben, Junio,
>
> Tonight, the testsuite passed with a couple of 'unexpected passes', viz:
>
> In the first case, t3421-*.sh, git bisect fingered commit f32ec670
> ("git-rebase--merge: don't include absent parent as a base", 20-04-2016).
>
> ATB,
> Ramsay Jones
>
Yep,
These know breakages:
ok 50 - rebase -m --onto --root
ok 54 - rebase -m without --onto --root with disjoint history
Have to do with rebasing a root/orphan branch with the -m flag,
which defaults to -- merge=recursive, which is the case the patch fixed.
Here are the necessary changes:
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -253,7 +253,7 @@ test_run_rebase () {
"
}
test_run_rebase success ''
-test_run_rebase failure -m
+test_run_rebase success -m
test_run_rebase success -i
test_run_rebase success -p
@@ -268,7 +268,7 @@ test_run_rebase () {
"
}
test_run_rebase success ''
-test_run_rebase failure -m
+test_run_rebase success -m
test_run_rebase success -i
test_run_rebase failure -p
--
2.8.1.211.g8e54d77
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make test Unexpected passes
On Fri, Apr 22, 2016 at 1:05 PM, Ramsay Jones
wrote:
> Hi Ben, Junio,
>
> In the second case, t6036-*.sh, git bisect fingered commit b61f9d6e
> ("ll-merge: use a longer conflict marker for internal merge", 14-04-2016).
Yeah, the t6036 testcase 'git detects conflict w/
criss-cross+contrived resolution' could be made to pass by tweaking
the conflict markers. In fact, any tweak would make it appear to
pass, but the test could be updated to still fail by updating the
contrived resolution of a previous merge to use the newer conflict
marker style as well.
The test is kind of fragile in this way, and is really there just to
document this slightly weird and never seen in practice corner case.
I don't think we'll ever fix the underlying "problem"; not even sure
if it's possible without fundamentally re-thinking our merge strategy
altogether. I just thought that when I was writing all those tests
that documenting this particular behavior as a testcase had some
value, but if the conflict markers are going to be updated
periodically, then the cost of the test may outweigh its value and we
should just toss this one test from that file.
Elijah
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
make test Unexpected passes
Hi Ben, Junio,
Tonight, the testsuite passed with a couple of 'unexpected passes', viz:
$ tail -17 ptest-out
[13:24:29]
All tests successful.
Test Summary Report
---
t3421-rebase-topology-linear.sh (Wstat: 0 Tests: 76 Failed: 0)
TODO passed: 50, 54
t6036-recursive-corner-cases.sh (Wstat: 0 Tests: 22 Failed: 0)
TODO passed: 11
Files=746, Tests=13515, 445 wallclock secs ( 3.83 usr 0.61 sys + 52.78 cusr
27.89 csys = 85.11 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory `/home/ramsay/git/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory `/home/ramsay/git/t'
make[1]: Leaving directory `/home/ramsay/git/t'
$
In the first case, t3421-*.sh, git bisect fingered commit f32ec670
("git-rebase--merge: don't include absent parent as a base", 20-04-2016).
In the second case, t6036-*.sh, git bisect fingered commit b61f9d6e
("ll-merge: use a longer conflict marker for internal merge", 14-04-2016).
I won't have any time tonight to look into this any further (are these
false positives?), so I thought I would just make sure you were aware
of these 'unexpected passes'.
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
On Fri, Mar 25, 2016 at 2:15 PM, Pranit Bauva wrote: > On Fri, Mar 25, 2016 at 10:34 PM, Eric Sunshine > wrote: >> That works too, simplifying the overall implementation, and >> eliminating the need for the introductory patch which moves >> 'test_set_editor' into each test. > > Wouldn't it be cleaner if the introductory patch contain: > 1. using write_script() > 2. grep the output to a file > 3. test for no of lines=1 in required tests > ? You could do that. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
On Fri, Mar 25, 2016 at 10:34 PM, Eric Sunshine wrote: > That works too, simplifying the overall implementation, and > eliminating the need for the introductory patch which moves > 'test_set_editor' into each test. Wouldn't it be cleaner if the introductory patch contain: 1. using write_script() 2. grep the output to a file 3. test for no of lines=1 in required tests ? Then the patch 3/3 >> And if you insist on doing the line counting in the editor script, then >> why redirecting grep's output and 'wc -l' separately, why not 'grep -c'? > > Ugh, I utterly forgot about -c. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
On Fri, Mar 25, 2016 at 10:46 AM, SZEDER Gábor wrote: > Quoting Eric Sunshine : >> On Thu, Mar 24, 2016 at 7:00 AM, SZEDER Gábor wrote: -cat >check-for-diff <>>> -#!$SHELL_PATH -exec grep '^diff --git' "\$1" +write_script "check-for-diff" <<-\EOF && +grep '^diff --git' "$1" >out && +test $(wc -l >> >>> Our test lib offers the test_line_count helper function, which >>> outputs a helpful error message in case the number of lines do not >>> match. >> >> test_line_count() was mentioned in [2], however, this line counting is >> done in the fake "editor" script, not in the test script proper, so >> the spelled-out form $(wc ...) was proposed[2]. > > Ah, yes, of course. > > But then the question is: why is the line counting in the editor script > in the first place? > > By redirecting grep's output to a file in the editor script, like this > patch wanted to, we can count the lines in the test script itself after > 'git commit' finished. This way we could use test_line_count, with > all its error reporting benefits, and we could use the same editor > script for all tests. That works too, simplifying the overall implementation, and eliminating the need for the introductory patch which moves 'test_set_editor' into each test. > And if you insist on doing the line counting in the editor script, then > why redirecting grep's output and 'wc -l' separately, why not 'grep -c'? Ugh, I utterly forgot about -c. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
On Fri, Mar 25, 2016 at 8:16 PM, SZEDER Gábor wrote: > By redirecting grep's output to a file in the editor script, like this > patch wanted to, we can count the lines in the test script itself after > 'git commit' finished. This way we could use test_line_count, with > all its error reporting benefits, and we could use the same editor > script for all tests. > > And if you insist on doing the line counting in the editor script, then > why redirecting grep's output and 'wc -l' separately, why not 'grep -c'? Nice way. Hadn't thought of this before. This will also eliminate having 2 different scripts to test for 1 line and other to test for 2 lines. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
Quoting Eric Sunshine : On Thu, Mar 24, 2016 at 7:00 AM, SZEDER Gábor wrote: diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh index 2ddf28c..cf95efb 100755 --- a/t/t7507-commit-verbose.sh +++ b/t/t7507-commit-verbose.sh @@ -3,12 +3,11 @@ test_description='verbose commit template' . ./test-lib.sh -cat >check-for-diff
Re: [PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
On Fri, Mar 25, 2016 at 11:54 AM, Eric Sunshine wrote: > On Fri, Mar 25, 2016 at 2:06 AM, Pranit Bauva wrote: >> On Fri, Mar 25, 2016 at 5:27 AM, Eric Sunshine >> wrote: >>> Agreed that this needs proper justification in the commit message. >>> And, the justification is to make each test more self-contained, >>> particularly because a subsequent patch will introduce a second fake >>> "editor", and by making tests responsible for setting the editor they >>> need, they don't have to worry about bad interactions from "editors" >>> set by earlier tests[1][2]. >> >> This shou cadve mbe ave be ave be ave be ave be ave be ave be ave > > Hmm, yes, what you say makes perfect sense... er, wait... haha :) Sorry for this. My browser crashed and it sent out some crap (don't know how). I mean to say, "This should have been mentioned in the commit message that scope of editor is reduced so as to not worry about bad interactions from other "editors" which may be used after wards." > > -cat >check-for-diff < -#!$SHELL_PATH > -exec grep '^diff --git' "\$1" > +write_script "check-for-diff" <<-\EOF && > +grep '^diff --git' "$1" >out && > +test $(wc -l >>> Our test lib offers the test_line_count helper function, which outputs a helpful error message in case the number of lines do not match. >>> >>> test_line_count() was mentioned in [2], however, this line counting is >>> done in the fake "editor" script, not in the test script proper, so >>> the spelled-out form $(wc ...) was proposed[2]. >> >> I have a slight doubt regarding this. Can the functions from test-lib >> work in such scripts flawlessly? If yes, then its probably better to >> use them. > > Probably not easily, and it's not worth complicating the "editor" > script by trying to import the test_line_count() function. Sure! > chmod +x check-for-diff >>> >>> Drop the 'chmod' line; write_script() does this for you. >> >> I was unaware about this. I will drop it off. > > I thought I had mentioned it in the review in which I had suggested > using write_script() or one of the follow-up emails, but upon looking > back at those messages, I saw it was not so. It turns out that I was > probably thinking about a different patch review in which I had > mentioned dropping 'chmod'[1]. > > [1]: http://article.gmane.org/gmane.comp.version-control.git/288085/ Oh! I hadn't tested by removing chmod. I will try it for fun. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
On Fri, Mar 25, 2016 at 2:06 AM, Pranit Bauva wrote: > On Fri, Mar 25, 2016 at 5:27 AM, Eric Sunshine > wrote: >> Agreed that this needs proper justification in the commit message. >> And, the justification is to make each test more self-contained, >> particularly because a subsequent patch will introduce a second fake >> "editor", and by making tests responsible for setting the editor they >> need, they don't have to worry about bad interactions from "editors" >> set by earlier tests[1][2]. > > This shou cadve mbe ave be ave be ave be ave be ave be ave be ave Hmm, yes, what you say makes perfect sense... er, wait... -cat >check-for-diff <>>> -#!$SHELL_PATH -exec grep '^diff --git' "\$1" +write_script "check-for-diff" <<-\EOF && +grep '^diff --git' "$1" >out && +test $(wc -l >> >>> Our test lib offers the test_line_count helper function, which >>> outputs a helpful error message in case the number of lines do not >>> match. >> >> test_line_count() was mentioned in [2], however, this line counting is >> done in the fake "editor" script, not in the test script proper, so >> the spelled-out form $(wc ...) was proposed[2]. > > I have a slight doubt regarding this. Can the functions from test-lib > work in such scripts flawlessly? If yes, then its probably better to > use them. Probably not easily, and it's not worth complicating the "editor" script by trying to import the test_line_count() function. chmod +x check-for-diff >> >> Drop the 'chmod' line; write_script() does this for you. > > I was unaware about this. I will drop it off. I thought I had mentioned it in the review in which I had suggested using write_script() or one of the follow-up emails, but upon looking back at those messages, I saw it was not so. It turns out that I was probably thinking about a different patch review in which I had mentioned dropping 'chmod'[1]. [1]: http://article.gmane.org/gmane.comp.version-control.git/288085/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
On Fri, Mar 25, 2016 at 5:27 AM, Eric Sunshine wrote:
> On Thu, Mar 24, 2016 at 7:00 AM, SZEDER Gábor wrote:
>>> Also remove test_set_editor from global scope and use it in whichever
>>> test it is required.
>>
>> Why?
>>
>> test_set_editor sets and exports shell variables. Since you don't
>> invoke test_set_editor in a subshell, after the first invocation the
>> editor will be part of the global scope anyway.
>
> Agreed that this needs proper justification in the commit message.
> And, the justification is to make each test more self-contained,
> particularly because a subsequent patch will introduce a second fake
> "editor", and by making tests responsible for setting the editor they
> need, they don't have to worry about bad interactions from "editors"
> set by earlier tests[1][2].
This shou cadve mbe ave be ave be ave be ave be ave be ave be ave
> Another issue is that the commit message is backward. The subject
> ("t7507-commit-verbose: make test suite use write_script") tries to
> sell this as primarily being about write_script(), but the real gist
> of the patch is that it is making each test more self-contained, and
> the subject should say so. The write_script() bit is just a minor
> aside which can be introduced with a "While here let's use
> write_script to..." at the end of the commit message.
>
>>> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
>>> index 2ddf28c..cf95efb 100755
>>> --- a/t/t7507-commit-verbose.sh
>>> +++ b/t/t7507-commit-verbose.sh
>>> @@ -3,12 +3,11 @@
>>> test_description='verbose commit template'
>>> . ./test-lib.sh
>>>
>>> -cat >check-for-diff <>> -#!$SHELL_PATH
>>> -exec grep '^diff --git' "\$1"
>>> +write_script "check-for-diff" <<-\EOF &&
>>> +grep '^diff --git' "$1" >out &&
>>> +test $(wc -l >
>> Our test lib offers the test_line_count helper function, which
>> outputs a helpful error message in case the number of lines do not
>> match.
>
> test_line_count() was mentioned in [2], however, this line counting is
> done in the fake "editor" script, not in the test script proper, so
> the spelled-out form $(wc ...) was proposed[2].
I have a slight doubt regarding this. Can the functions from test-lib
work in such scripts flawlessly? If yes, then its probably better to
use them.
>> The original didn't check the number of lines. This change is not
>> mentioned at all in the commit message.
>
> Right, and this particular change really belongs in patch 3/3 where
> it's needed[2], and should be properly explained by the 3/3 commit
> message.
Sure! It should have been mentioned.
>>> EOF
>>> chmod +x check-for-diff
>
> Drop the 'chmod' line; write_script() does this for you.
I was unaware about this. I will drop it off.
>>> -test_set_editor "$PWD/check-for-diff"
>>>
>>> cat >message <<'EOF'
>>> subject
>
> [1]: http://article.gmane.org/gmane.comp.version-control.git/289329
> [2]: http://article.gmane.org/gmane.comp.version-control.git/289370
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
On Thu, Mar 24, 2016 at 7:00 AM, SZEDER Gábor wrote:
>> Also remove test_set_editor from global scope and use it in whichever
>> test it is required.
>
> Why?
>
> test_set_editor sets and exports shell variables. Since you don't
> invoke test_set_editor in a subshell, after the first invocation the
> editor will be part of the global scope anyway.
Agreed that this needs proper justification in the commit message.
And, the justification is to make each test more self-contained,
particularly because a subsequent patch will introduce a second fake
"editor", and by making tests responsible for setting the editor they
need, they don't have to worry about bad interactions from "editors"
set by earlier tests[1][2].
Another issue is that the commit message is backward. The subject
("t7507-commit-verbose: make test suite use write_script") tries to
sell this as primarily being about write_script(), but the real gist
of the patch is that it is making each test more self-contained, and
the subject should say so. The write_script() bit is just a minor
aside which can be introduced with a "While here let's use
write_script to..." at the end of the commit message.
>> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
>> index 2ddf28c..cf95efb 100755
>> --- a/t/t7507-commit-verbose.sh
>> +++ b/t/t7507-commit-verbose.sh
>> @@ -3,12 +3,11 @@
>> test_description='verbose commit template'
>> . ./test-lib.sh
>>
>> -cat >check-for-diff <> -#!$SHELL_PATH
>> -exec grep '^diff --git' "\$1"
>> +write_script "check-for-diff" <<-\EOF &&
>> +grep '^diff --git' "$1" >out &&
>> +test $(wc -l
> Our test lib offers the test_line_count helper function, which
> outputs a helpful error message in case the number of lines do not
> match.
test_line_count() was mentioned in [2], however, this line counting is
done in the fake "editor" script, not in the test script proper, so
the spelled-out form $(wc ...) was proposed[2].
> The original didn't check the number of lines. This change is not
> mentioned at all in the commit message.
Right, and this particular change really belongs in patch 3/3 where
it's needed[2], and should be properly explained by the 3/3 commit
message.
>> EOF
>> chmod +x check-for-diff
Drop the 'chmod' line; write_script() does this for you.
>> -test_set_editor "$PWD/check-for-diff"
>>
>> cat >message <<'EOF'
>> subject
[1]: http://article.gmane.org/gmane.comp.version-control.git/289329
[2]: http://article.gmane.org/gmane.comp.version-control.git/289370
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
> Also remove test_set_editor from global scope and use it in whichever
> test it is required.
Why?
test_set_editor sets and exports shell variables. Since you don't
invoke test_set_editor in a subshell, after the first invocation the
editor will be part of the global scope anyway.
Also missing signoff.
> ---
> t/t7507-commit-verbose.sh | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
> index 2ddf28c..cf95efb 100755
> --- a/t/t7507-commit-verbose.sh
> +++ b/t/t7507-commit-verbose.sh
> @@ -3,12 +3,11 @@
> test_description='verbose commit template'
> . ./test-lib.sh
>
> -cat >check-for-diff < -#!$SHELL_PATH
> -exec grep '^diff --git' "\$1"
> +write_script "check-for-diff" <<-\EOF &&
> +grep '^diff --git' "$1" >out &&
> +test $(wc -l EOF
> chmod +x check-for-diff
> -test_set_editor "$PWD/check-for-diff"
>
> cat >message <<'EOF'
> subject
> @@ -23,6 +22,7 @@ test_expect_success 'setup' '
> '
>
> test_expect_success 'initial commit shows verbose diff' '
> + test_set_editor "$PWD/check-for-diff" &&
> git commit --amend -v
> '
>
> @@ -38,11 +38,13 @@ check_message() {
> }
>
> test_expect_success 'verbose diff is stripped out' '
> + test_set_editor "$PWD/check-for-diff" &&
> git commit --amend -v &&
> check_message message
> '
>
> test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '
> + test_set_editor "$PWD/check-for-diff" &&
> git config diff.mnemonicprefix true &&
> git commit --amend -v &&
> check_message message
> @@ -66,11 +68,13 @@ test_expect_success 'diff in message is retained without
> -v' '
> '
>
> test_expect_success 'diff in message is retained with -v' '
> + test_set_editor "$PWD/check-for-diff" &&
> git commit --amend -F diff -v &&
> check_message diff
> '
>
> test_expect_success 'submodule log is stripped out too with -v' '
> + test_set_editor "$PWD/check-for-diff" &&
> git config diff.submodule log &&
> git submodule add ./. sub &&
> git commit -m "sub added" &&
>
> --
> https://github.com/git/git/pull/218
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script
Also remove test_set_editor from global scope and use it in whichever test it is required. --- t/t7507-commit-verbose.sh | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh index 2ddf28c..cf95efb 100755 --- a/t/t7507-commit-verbose.sh +++ b/t/t7507-commit-verbose.sh @@ -3,12 +3,11 @@ test_description='verbose commit template' . ./test-lib.sh -cat >check-for-diff
Re: Bug when doing make test using root user
On Fri, Jun 12, 2015 at 5:43 PM, Jean-Yves LENHOF wrote: > Hi, > > I tried to compile git 2.4.3 using root on a server. It failed on test 41 of > t0302-credential-store.sh > In fact even if we remove read access on a directory, root still can acces > this directory. > Using a not privilegied user make the test work. > Perhaps the test should be adapted to this corner case. > Trace below. Right, the test should have the SANITY prereq. Thanks for reporting. Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bug when doing make test using root user
Hi, I tried to compile git 2.4.3 using root on a server. It failed on test 41 of t0302-credential-store.sh In fact even if we remove read access on a directory, root still can acces this directory. Using a not privilegied user make the test work. Perhaps the test should be adapted to this corner case. Trace below. Have fun, JYL *** t0302-credential-store.sh *** ok 1 - helper (store) has no existing data ok 2 - helper (store) stores password ok 3 - helper (store) can retrieve password ok 4 - helper (store) requires matching protocol ok 5 - helper (store) requires matching host ok 6 - helper (store) requires matching username ok 7 - helper (store) requires matching path ok 8 - helper (store) can forget host ok 9 - helper (store) can store multiple users ok 10 - helper (store) can forget user ok 11 - helper (store) remembers other user ok 12 - when xdg file does not exist, xdg file not created ok 13 - setup xdg file ok 14 - helper (store) has no existing data ok 15 - helper (store) stores password ok 16 - helper (store) can retrieve password ok 17 - helper (store) requires matching protocol ok 18 - helper (store) requires matching host ok 19 - helper (store) requires matching username ok 20 - helper (store) requires matching path ok 21 - helper (store) can forget host ok 22 - helper (store) can store multiple users ok 23 - helper (store) can forget user ok 24 - helper (store) remembers other user ok 25 - when xdg file exists, home file not created ok 26 - setup custom xdg file ok 27 - helper (store) has no existing data ok 28 - helper (store) stores password ok 29 - helper (store) can retrieve password ok 30 - helper (store) requires matching protocol ok 31 - helper (store) requires matching host ok 32 - helper (store) requires matching username ok 33 - helper (store) requires matching path ok 34 - helper (store) can forget host ok 35 - helper (store) can store multiple users ok 36 - helper (store) can forget user ok 37 - helper (store) remembers other user ok 38 - if custom xdg file exists, home and xdg files not created ok 39 - get: use home file if both home and xdg files have matches ok 40 - get: use xdg file if home file has no matches not ok 41 - get: use xdg file if home file is unreadable # # echo "https://home-user:home-p...@example.com"; >"$HOME/.git-credentials" && # chmod -r "$HOME/.git-credentials" && # mkdir -p "$HOME/.config/git" && # echo "https://xdg-user:xdg-p...@example.com"; >"$HOME/.config/git/credentials" && # check fill store <<-\EOF # protocol=https # host=example.com # -- # protocol=https # host=example.com # username=xdg-user # password=xdg-pass # -- # EOF # ok 42 - store: if both xdg and home files exist, only store in home file ok 43 - erase: erase matching credentials from both xdg and home files # failed 1 among 43 test(s) 1..43 make[2]: *** [t0302-credential-store.sh] Error 1 make[2]: Leaving directory `/root/git-2.4.3/t' make[1]: *** [test] Error 2 make[1]: Leaving directory `/root/git-2.4.3/t' make: *** [test] Error 2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] t1700: make test pass with index-v4
On 03/20, Junio C Hamano wrote: > Thomas Gummerer writes: > > > The different index versions have different sha-1 checksums. Those > > checksums are checked in t1700, which makes it fail when the test suite > > is run with TEST_GIT_INDEX_VERSION=4. Fix it. > > > > Signed-off-by: Thomas Gummerer > > --- > >> An updated patch to mention "when run with TEST_GIT_INDEX_VERSION=4" > >> in the log message was what I was asking for ;-) > > > > Sorry I didn't catch that. Here it is. > > Thanks. Is this the only one that fails with v4? Yes, with this patch the test suite passes with TEST_GIT_INDEX_VERSION set. I've now tried to set the GIT_TEST_SPLIT_INDEX environment variable and run the tests, and quite a few seem to fail for both v3 and v4. I'm looking into that now, but that seems to be unrelated to index v4, and I'm not sure yet if I'm testing the right thing. > > > > > > t/t1700-split-index.sh | 15 --- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh > > index 94fb473..92f7298 100755 > > --- a/t/t1700-split-index.sh > > +++ b/t/t1700-split-index.sh > > @@ -10,9 +10,18 @@ sane_unset GIT_TEST_SPLIT_INDEX > > test_expect_success 'enable split index' ' > > git update-index --split-index && > > test-dump-split-index .git/index >actual && > > + indexversion=$(test-index-version <.git/index) && > > + if test "$indexversion" = "4" > > + then > > + own=432ef4b63f32193984f339431fd50ca796493569 > > + base=508851a7f0dfa8691e9f69c7f055865389012491 > > + else > > + own=8299b0bcd1ac364e5f1d7768efb62fa2da79a339 > > + base=39d890139ee5356c7ef572216cebcd27aa41f9df > > + fi && > > cat >expect < > -own 8299b0bcd1ac364e5f1d7768efb62fa2da79a339 > > -base 39d890139ee5356c7ef572216cebcd27aa41f9df > > +own $own > > +base $base > > replacements: > > deletions: > > EOF > > @@ -30,7 +39,7 @@ EOF > > > > test-dump-split-index .git/index | sed "/^own/d" >actual && > > cat >expect < > -base 39d890139ee5356c7ef572216cebcd27aa41f9df > > +base $base > > 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one > > replacements: > > deletions: -- Thomas Gummerer -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] t1700: make test pass with index-v4
Thomas Gummerer writes: > The different index versions have different sha-1 checksums. Those > checksums are checked in t1700, which makes it fail when the test suite > is run with TEST_GIT_INDEX_VERSION=4. Fix it. > > Signed-off-by: Thomas Gummerer > --- >> An updated patch to mention "when run with TEST_GIT_INDEX_VERSION=4" >> in the log message was what I was asking for ;-) > > Sorry I didn't catch that. Here it is. Thanks. Is this the only one that fails with v4? > > > t/t1700-split-index.sh | 15 --- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh > index 94fb473..92f7298 100755 > --- a/t/t1700-split-index.sh > +++ b/t/t1700-split-index.sh > @@ -10,9 +10,18 @@ sane_unset GIT_TEST_SPLIT_INDEX > test_expect_success 'enable split index' ' > git update-index --split-index && > test-dump-split-index .git/index >actual && > + indexversion=$(test-index-version <.git/index) && > + if test "$indexversion" = "4" > + then > + own=432ef4b63f32193984f339431fd50ca796493569 > + base=508851a7f0dfa8691e9f69c7f055865389012491 > + else > + own=8299b0bcd1ac364e5f1d7768efb62fa2da79a339 > + base=39d890139ee5356c7ef572216cebcd27aa41f9df > + fi && > cat >expect < -own 8299b0bcd1ac364e5f1d7768efb62fa2da79a339 > -base 39d890139ee5356c7ef572216cebcd27aa41f9df > +own $own > +base $base > replacements: > deletions: > EOF > @@ -30,7 +39,7 @@ EOF > > test-dump-split-index .git/index | sed "/^own/d" >actual && > cat >expect < -base 39d890139ee5356c7ef572216cebcd27aa41f9df > +base $base > 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0one > replacements: > deletions: -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] t1700: make test pass with index-v4
The different index versions have different sha-1 checksums. Those checksums are checked in t1700, which makes it fail when the test suite is run with TEST_GIT_INDEX_VERSION=4. Fix it. Signed-off-by: Thomas Gummerer --- > An updated patch to mention "when run with TEST_GIT_INDEX_VERSION=4" > in the log message was what I was asking for ;-) Sorry I didn't catch that. Here it is. t/t1700-split-index.sh | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index 94fb473..92f7298 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -10,9 +10,18 @@ sane_unset GIT_TEST_SPLIT_INDEX test_expect_success 'enable split index' ' git update-index --split-index && test-dump-split-index .git/index >actual && + indexversion=$(test-index-version <.git/index) && + if test "$indexversion" = "4" + then + own=432ef4b63f32193984f339431fd50ca796493569 + base=508851a7f0dfa8691e9f69c7f055865389012491 + else + own=8299b0bcd1ac364e5f1d7768efb62fa2da79a339 + base=39d890139ee5356c7ef572216cebcd27aa41f9df + fi && cat >expectexpect
Re: [PATCH] t1700: make test pass with index-v4
Thomas Gummerer writes: > On 03/20, Junio C Hamano wrote: >> Thomas Gummerer writes: >> >> > The different index versions have different sha-1 checksums. Those >> > checksums are checked in t1700, which makes it fail when run with index >> > v4. Fix it. >> >> I am more interested to see how you managed to use index v4 in the >> tests be described next to "when run with index v4". I thought we >> were controling these things fairly tightly (e.g. we disable hooks, >> move $HOME to avoid getting affected by your personal settings, >> etc.). > > The tests can be run with index-v4 by setting TEST_GIT_INDEX_VERSION > in config.mak. This configuration was introduced in 5d9fc88 test-lib: > allow setting the index format version. An updated patch to mention "when run with TEST_GIT_INDEX_VERSION=4" in the log message was what I was asking for ;-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t1700: make test pass with index-v4
On 03/20, Junio C Hamano wrote: > Thomas Gummerer writes: > > > The different index versions have different sha-1 checksums. Those > > checksums are checked in t1700, which makes it fail when run with index > > v4. Fix it. > > I am more interested to see how you managed to use index v4 in the > tests be described next to "when run with index v4". I thought we > were controling these things fairly tightly (e.g. we disable hooks, > move $HOME to avoid getting affected by your personal settings, > etc.). The tests can be run with index-v4 by setting TEST_GIT_INDEX_VERSION in config.mak. This configuration was introduced in 5d9fc88 test-lib: allow setting the index format version. > Thanks. > > > > Signed-off-by: Thomas Gummerer > > --- > > t/t1700-split-index.sh | 15 --- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh > > index 94fb473..92f7298 100755 > > --- a/t/t1700-split-index.sh > > +++ b/t/t1700-split-index.sh > > @@ -10,9 +10,18 @@ sane_unset GIT_TEST_SPLIT_INDEX > > test_expect_success 'enable split index' ' > > git update-index --split-index && > > test-dump-split-index .git/index >actual && > > + indexversion=$(test-index-version <.git/index) && > > + if test "$indexversion" = "4" > > + then > > + own=432ef4b63f32193984f339431fd50ca796493569 > > + base=508851a7f0dfa8691e9f69c7f055865389012491 > > + else > > + own=8299b0bcd1ac364e5f1d7768efb62fa2da79a339 > > + base=39d890139ee5356c7ef572216cebcd27aa41f9df > > + fi && > > cat >expect < > -own 8299b0bcd1ac364e5f1d7768efb62fa2da79a339 > > -base 39d890139ee5356c7ef572216cebcd27aa41f9df > > +own $own > > +base $base > > replacements: > > deletions: > > EOF > > @@ -30,7 +39,7 @@ EOF > > > > test-dump-split-index .git/index | sed "/^own/d" >actual && > > cat >expect < > -base 39d890139ee5356c7ef572216cebcd27aa41f9df > > +base $base > > 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one > > replacements: > > deletions: -- Thomas Gummerer -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t1700: make test pass with index-v4
Thomas Gummerer writes: > The different index versions have different sha-1 checksums. Those > checksums are checked in t1700, which makes it fail when run with index > v4. Fix it. I am more interested to see how you managed to use index v4 in the tests be described next to "when run with index v4". I thought we were controling these things fairly tightly (e.g. we disable hooks, move $HOME to avoid getting affected by your personal settings, etc.). Thanks. > Signed-off-by: Thomas Gummerer > --- > t/t1700-split-index.sh | 15 --- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh > index 94fb473..92f7298 100755 > --- a/t/t1700-split-index.sh > +++ b/t/t1700-split-index.sh > @@ -10,9 +10,18 @@ sane_unset GIT_TEST_SPLIT_INDEX > test_expect_success 'enable split index' ' > git update-index --split-index && > test-dump-split-index .git/index >actual && > + indexversion=$(test-index-version <.git/index) && > + if test "$indexversion" = "4" > + then > + own=432ef4b63f32193984f339431fd50ca796493569 > + base=508851a7f0dfa8691e9f69c7f055865389012491 > + else > + own=8299b0bcd1ac364e5f1d7768efb62fa2da79a339 > + base=39d890139ee5356c7ef572216cebcd27aa41f9df > + fi && > cat >expect < -own 8299b0bcd1ac364e5f1d7768efb62fa2da79a339 > -base 39d890139ee5356c7ef572216cebcd27aa41f9df > +own $own > +base $base > replacements: > deletions: > EOF > @@ -30,7 +39,7 @@ EOF > > test-dump-split-index .git/index | sed "/^own/d" >actual && > cat >expect < -base 39d890139ee5356c7ef572216cebcd27aa41f9df > +base $base > 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0one > replacements: > deletions: -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t1700: make test pass with index-v4
The different index versions have different sha-1 checksums. Those checksums are checked in t1700, which makes it fail when run with index v4. Fix it. Signed-off-by: Thomas Gummerer --- t/t1700-split-index.sh | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index 94fb473..92f7298 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -10,9 +10,18 @@ sane_unset GIT_TEST_SPLIT_INDEX test_expect_success 'enable split index' ' git update-index --split-index && test-dump-split-index .git/index >actual && + indexversion=$(test-index-version <.git/index) && + if test "$indexversion" = "4" + then + own=432ef4b63f32193984f339431fd50ca796493569 + base=508851a7f0dfa8691e9f69c7f055865389012491 + else + own=8299b0bcd1ac364e5f1d7768efb62fa2da79a339 + base=39d890139ee5356c7ef572216cebcd27aa41f9df + fi && cat >expectexpect
Re: 'make test' fails in pu
Jeff King writes: > Sometimes a breakage in pu is surprising (e.g., it breaks only on a > platform that the maintainer does not run "make test" on) and we would > want to know about it. But sometimes it is merely that there is a > work-in-progress. And it probably requires a human to tell the > difference. > > So no, I do not think automatically mailing on test failures in pu is a > good idea. Manually peeking at them and sending fixes before the series > is merged to next _is_ very much encouraged, though. :) Thanks, that is exactly what people saw. From time to time, I queue a topic that does not pass the tests on 'pu', hoping that somebody sufficiently interested would step in to collaborate with the author of the topic to move things forward when the breakage looks simple enough, and sometimes that original author happens to be me. This case, it turns out that the breakage is not so simple, though. Inside the rename detection logic, I want to peek the rename source array to decide which deletion filepair to keep, but rename source array itself has pointers to the original filepairs the loop wants to free, so the WIP code was peeking into a freed piece memory X-<. > Unlike "pu", "next" and "master" should never fail tests (I think that > Junio will not push them out if the tests have failed on his system). So > failures there are much more likely to be interesting platform bugs (but > of course, testing "pu" is still encouraged, as we may catch problems). True. I do not mind automated tests on 'next', and testing 'pu' and helping the topic to move forward is very much encouraged, but sending test results on 'pu' blindly is often more noise than it is worth. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 'make test' fails in pu
For the record, that commit also sporadically breaks test 3910 on my system (mentioning since it's not on the list) On Tue, Feb 17, 2015 at 12:55 AM, Jeff King wrote: > On Tue, Feb 17, 2015 at 09:39:17AM +0100, Dennis Kaarsemaker wrote: > >> Make test has been failing for 'pu' yesterday for and today at >> t4016-diff-quote.sh. Full log: >> http://ci.kaarsemaker.net/git/refs/heads/pu/1df29c71a731c679de9055ae5e407f3a4e18740a/artefact/test/log >> >> I noticed this a few times before and it tends to get fixed again >> relatively quickly. So I'm wondering: >> >> - Should I even mention that it's failing, or is that just useless >> noise? >> - If I should report this, I could also make my testing thing send >> mails. Would that be useful? > > If you bisect this, it turns up commit 30cd8f94f, which says: > > WIP: diff-b-m > > [...] > > This update is still broken and breaks a handful of tests: > > 4016 4023 4047 4130 6022 6031 6032 9300 9200 9300 9350 > > Sometimes a breakage in pu is surprising (e.g., it breaks only on a > platform that the maintainer does not run "make test" on) and we would > want to know about it. But sometimes it is merely that there is a > work-in-progress. And it probably requires a human to tell the > difference. > > So no, I do not think automatically mailing on test failures in pu is a > good idea. Manually peeking at them and sending fixes before the series > is merged to next _is_ very much encouraged, though. :) > > Unlike "pu", "next" and "master" should never fail tests (I think that > Junio will not push them out if the tests have failed on his system). So > failures there are much more likely to be interesting platform bugs (but > of course, testing "pu" is still encouraged, as we may catch problems). > > But even for "next", I would say blind automated emails are not nearly > as useful as a human who has looked at the problem (and especially > bisected). > > -Peff > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 'make test' fails in pu
On Tue, Feb 17, 2015 at 09:39:17AM +0100, Dennis Kaarsemaker wrote: > Make test has been failing for 'pu' yesterday for and today at > t4016-diff-quote.sh. Full log: > http://ci.kaarsemaker.net/git/refs/heads/pu/1df29c71a731c679de9055ae5e407f3a4e18740a/artefact/test/log > > I noticed this a few times before and it tends to get fixed again > relatively quickly. So I'm wondering: > > - Should I even mention that it's failing, or is that just useless > noise? > - If I should report this, I could also make my testing thing send > mails. Would that be useful? If you bisect this, it turns up commit 30cd8f94f, which says: WIP: diff-b-m [...] This update is still broken and breaks a handful of tests: 4016 4023 4047 4130 6022 6031 6032 9300 9200 9300 9350 Sometimes a breakage in pu is surprising (e.g., it breaks only on a platform that the maintainer does not run "make test" on) and we would want to know about it. But sometimes it is merely that there is a work-in-progress. And it probably requires a human to tell the difference. So no, I do not think automatically mailing on test failures in pu is a good idea. Manually peeking at them and sending fixes before the series is merged to next _is_ very much encouraged, though. :) Unlike "pu", "next" and "master" should never fail tests (I think that Junio will not push them out if the tests have failed on his system). So failures there are much more likely to be interesting platform bugs (but of course, testing "pu" is still encouraged, as we may catch problems). But even for "next", I would say blind automated emails are not nearly as useful as a human who has looked at the problem (and especially bisected). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
'make test' fails in pu
Make test has been failing for 'pu' yesterday for and today at t4016-diff-quote.sh. Full log: http://ci.kaarsemaker.net/git/refs/heads/pu/1df29c71a731c679de9055ae5e407f3a4e18740a/artefact/test/log I noticed this a few times before and it tends to get fixed again relatively quickly. So I'm wondering: - Should I even mention that it's failing, or is that just useless noise? - If I should report this, I could also make my testing thing send mails. Would that be useful? -- Dennis Kaarsemaker http://www.kaarsemaker.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t3200-branch.sh number 102 fails when run under make test
On Wed, 2014-07-09 at 16:54 -0400, Jeff King wrote:
> On Wed, Jul 09, 2014 at 08:37:51PM +, Keller, Jacob E wrote:
>
> > I recently cloned the master branch of the git repo, and when I ran make
> > test, it fails on test 102 of the t3200-branch.sh test cases.
>
> Just a guess, but try reverting 745224e (refs.c: SSE2 optimizations for
> check_refname_component, 2014-06-18).
>
> That commit causes some weird memory accesses that only show up under
> certain conditions[1]. There's a possible fix that is not yet applied,
> but reverting should be an easy way to test.
>
> -Peff
>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/252881
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Yes, performing the revert appears to fix the issue. Hopefully the
proposed fix also works.
Thanks,
Jake
N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf
Re: t3200-branch.sh number 102 fails when run under make test
On Wed, 2014-07-09 at 13:37 -0700, Jacob E Keller wrote:
> Hello,
>
> I recently cloned the master branch of the git repo, and when I ran make
> test, it fails on test 102 of the t3200-branch.sh test cases.
>
> not ok 102 - tracking with unexpected .fetch refspec
> #
> # rm -rf a b c d &&
> # git init a &&
> # (
> # cd a &&
> # test_commit a
> # ) &&
> # git init b &&
> # (
> # cd b &&
> # test_commit b
> # ) &&
> # git init c &&
> # (
> # cd c &&
> # test_commit c &&
> # git remote add a ../a &&
> # git remote add b ../b &&
> # git fetch --all
> # ) &&
> # git init d &&
> # (
> # cd d &&
> # git remote add c ../c &&
> # git config remote.c.fetch
> "+refs/remotes/*:refs/remotes/*" &&
> # git fetch c &&
> # git branch --track local/a/master remotes/a/master &&
> # test "$(git config branch.local/a/master.remote)" =
> "c" &&
> # test "$(git config branch.local/a/master.merge)" =
> "refs/remotes/a/master" &&
> # git rev-parse --verify a >expect &&
> # git rev-parse --verify local/a/master >actual &&
> # test_cmp expect actual
> # )
> #
> # failed 1 among 102 test(s)
> 1..102
>
> However, when I run the test file manually it passes. I am currently
> running through a verbose output test run to see if I can find more
> useful output..
>
> For reference, the commit I am testing against is:
>
> 72c779457cd7 ("line-log: use commit_list_append() instead of duplicating its
> code")
>
> Thanks,
> Jake
I ran the test wit the GIT_TEST_OPS set to --verbose, and the output I got is:
expecting success:
rm -rf a b c d &&
git init a &&
(
cd a &&
test_commit a
) &&
git init b &&
(
cd b &&
test_commit b
) &&
git init c &&
(
cd c &&
test_commit c &&
git remote add a ../a &&
git remote add b ../b &&
git fetch --all
) &&
git init d &&
(
cd d &&
git remote add c ../c &&
git config remote.c.fetch "+refs/remotes/*:refs/remotes/*" &&
git fetch c &&
git branch --track local/a/master remotes/a/master &&
test "$(git config branch.local/a/master.remote)" = "c" &&
test "$(git config branch.local/a/master.merge)" =
"refs/remotes/a/master" &&
git rev-parse --verify a >expect &&
git rev-parse --verify local/a/master >actual &&
test_cmp expect actual
)
Initialized empty Git repository in /home/jekeller/git/git/t/trash
directory.t3200-branch/a/.git/
[master (root-commit) ce450c7] a
Author: A U Thor
1 file changed, 1 insertion(+)
create mode 100644 a.t
Initialized empty Git repository in /home/jekeller/git/git/t/trash
directory.t3200-branch/b/.git/
[master (root-commit) 19acec0] b
Author: A U Thor
1 file changed, 1 insertion(+)
create mode 100644 b.t
Initialized empty Git repository in /home/jekeller/git/git/t/trash
directory.t3200-branch/c/.git/
[master (root-commit) ea1ac38] c
Author: A U Thor
1 file changed, 1 insertion(+)
create mode 100644 c.t
fatal: Invalid refspec '+refs/heads/*:refs/remotes/b/*'
not ok 102 - tracking with unexpected .fetch refspec
#
# rm -rf a b c d &&
# git init a &&
# (
# cd a &&
# test_commit a
# ) &&
# git init b &&
# (
# cd b &&
# test_commit b
# ) &&
# git init c &&
#
Re: t3200-branch.sh number 102 fails when run under make test
On Wed, Jul 09, 2014 at 08:37:51PM +, Keller, Jacob E wrote: > I recently cloned the master branch of the git repo, and when I ran make > test, it fails on test 102 of the t3200-branch.sh test cases. Just a guess, but try reverting 745224e (refs.c: SSE2 optimizations for check_refname_component, 2014-06-18). That commit causes some weird memory accesses that only show up under certain conditions[1]. There's a possible fix that is not yet applied, but reverting should be an easy way to test. -Peff [1] http://thread.gmane.org/gmane.comp.version-control.git/252881 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
t3200-branch.sh number 102 fails when run under make test
Hello,
I recently cloned the master branch of the git repo, and when I ran make
test, it fails on test 102 of the t3200-branch.sh test cases.
not ok 102 - tracking with unexpected .fetch refspec
#
# rm -rf a b c d &&
# git init a &&
# (
# cd a &&
# test_commit a
# ) &&
# git init b &&
# (
# cd b &&
# test_commit b
# ) &&
# git init c &&
# (
# cd c &&
# test_commit c &&
# git remote add a ../a &&
# git remote add b ../b &&
# git fetch --all
# ) &&
# git init d &&
# (
# cd d &&
# git remote add c ../c &&
# git config remote.c.fetch
"+refs/remotes/*:refs/remotes/*" &&
# git fetch c &&
# git branch --track local/a/master remotes/a/master &&
# test "$(git config branch.local/a/master.remote)" = "c"
&&
# test "$(git config branch.local/a/master.merge)" =
"refs/remotes/a/master" &&
# git rev-parse --verify a >expect &&
# git rev-parse --verify local/a/master >actual &&
# test_cmp expect actual
# )
#
# failed 1 among 102 test(s)
1..102
However, when I run the test file manually it passes. I am currently
running through a verbose output test run to see if I can find more
useful output..
For reference, the commit I am testing against is:
72c779457cd7 ("line-log: use commit_list_append() instead of duplicating its
code")
Thanks,
Jake
N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf
Re: [PATCH 1/3] Make test "using invalid commit with -C" more strict
Junio C Hamano wrote: >> Kacper Kornet wrote: >>> In the test 'using invalid commit with -C' git-commit would have failed >>> even if the -C option had been given the correct commit, as there was >>> nothing to commit. [...] > Also it would be much simpler to say "git commit --allow-empty". Sounds good. ;-) Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Make test "using invalid commit with -C" more strict
Jonathan Nieder writes: > Kacper Kornet wrote: > >> In the test 'using invalid commit with -C' git-commit would have failed >> even if the -C option had been given the correct commit, as there was >> nothing to commit. > > Good catch. > > [...] >> --- a/t/t7501-commit.sh >> +++ b/t/t7501-commit.sh >> @@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with >> --interactive' ' >> ' >> >> test_expect_success 'using invalid commit with -C' ' >> -test_must_fail git commit -C bogus >> +echo bong >file && >> +git add file && >> +test_must_fail git commit -C bogus && >> +git reset > > I guess to be pedantic this should say > > echo bong >file && > git add file && > test_when_finished "git reset --hard" && > test_must_fail git commit -C bogus > > to avoid interfering with later tests even when this one fails and > the && prevents the 'git reset' from being executed. Makes sense. Also it would be much simpler to say "git commit --allow-empty". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Make test "using invalid commit with -C" more strict
Kacper Kornet wrote: > In the test 'using invalid commit with -C' git-commit would have failed > even if the -C option had been given the correct commit, as there was > nothing to commit. Good catch. [...] > --- a/t/t7501-commit.sh > +++ b/t/t7501-commit.sh > @@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with > --interactive' ' > ' > > test_expect_success 'using invalid commit with -C' ' > - test_must_fail git commit -C bogus > + echo bong >file && > + git add file && > + test_must_fail git commit -C bogus && > + git reset I guess to be pedantic this should say echo bong >file && git add file && test_when_finished "git reset --hard" && test_must_fail git commit -C bogus to avoid interfering with later tests even when this one fails and the && prevents the 'git reset' from being executed. With or without that change, Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] Make test "using invalid commit with -C" more strict
In the test 'using invalid commit with -C' git-commit would have failed even if the -C option had been given the correct commit, as there was nothing to commit. Fix it by making sure there is always something to commit and git-commit fails because of the invalid commit provided to it. Signed-off-by: Kacper Kornet --- t/t7501-commit.sh | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index 99ce36f..699a603 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with --interactive' ' ' test_expect_success 'using invalid commit with -C' ' - test_must_fail git commit -C bogus + echo bong >file && + git add file && + test_must_fail git commit -C bogus && + git reset ' test_expect_success 'nothing to commit' ' -- 1.8.3.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] Make test "using invalid commit with -C" more strict
In the test 'using invalid commit with -C' git-commit would have failed even if the -C option had been given the correct commit, as there was nothing to commit. Fix it by making sure there is always something to commit and git-commit fails because of the invalid commit provided to it. Signed-off-by: Kacper Kornet --- t/t7501-commit.sh | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index 99ce36f..699a603 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with --interactive' ' ' test_expect_success 'using invalid commit with -C' ' - test_must_fail git commit -C bogus + echo bong >file && + git add file && + test_must_fail git commit -C bogus && + git reset ' test_expect_success 'nothing to commit' ' -- 1.8.3.4 >From 8ad7ef374e1b05cb73d6cf445c44f10e5ec36be3 Mon Sep 17 00:00:00 2001 From: Kacper Kornet Date: Sat, 24 Aug 2013 03:58:05 +0100 Subject: [PATCH 2/3] t/t3701-add-interactive.sh: Add PERL prerequisite The test 'patch mode ignores unmerged entries' uses git-add -p, so it depends on the perl code. Signed-off-by: Kacper Kornet --- t/t3701-add-interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 9fab25c..8514220 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -330,7 +330,7 @@ test_expect_success PERL 'split hunk "add -p (edit)"' ' ! grep "^+15" actual ' -test_expect_success 'patch mode ignores unmerged entries' ' +test_expect_success PERL 'patch mode ignores unmerged entries' ' git reset --hard && test_commit conflict && test_commit non-conflict && -- 1.8.3.4 >From 14f135b178bfaba67653bc3bedf65ec45e38bc76 Mon Sep 17 00:00:00 2001 From: Kacper Kornet Date: Sat, 24 Aug 2013 04:01:02 +0100 Subject: [PATCH 3/3] t/t7106-reset-unborn-branch.sh: Add PERL prerequisite The test 'reset -p' uses git-reset -p, so it depends on the perl code. Signed-off-by: Kacper Kornet --- t/t7106-reset-unborn-branch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh index 8062cf5..499cd88 100755 --- a/t/t7106-reset-unborn-branch.sh +++ b/t/t7106-reset-unborn-branch.sh @@ -27,7 +27,7 @@ test_expect_success 'reset $file' ' test "$(git ls-files)" = "b" ' -test_expect_success 'reset -p' ' +test_expect_success PERL 'reset -p' ' rm .git/index && git add a && echo y | git reset -p && -- 1.8.3.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] make test output coloring more intuitive
On Thu, Dec 20, 2012 at 7:21 PM, Junio C Hamano wrote: > Jeff King writes: >>> Good point, I forgot to check what it looked like with -v. Since this >>> series is already on v6, is there a more lightweight way of addressing >>> this tiny tweak than sending v7? >> >> It is ultimately up to Junio, but I suspect he would be OK if you just >> reposted patch 4/7 with the above squashed. Or even just said "I like >> this, please squash it into patch 4 (change info messages from >> yellow/brown to bold cyan). > > Surely; as long as the series is not in 'next', the change to be > squashed is not too big and it is not too much work (and in this > case it certainly is not). OK. > I actually wonder if "skipped test in bold blue" and "known breakage > in bold yellow" should also lose the boldness. Errors and warnings > in bold are good, but I would say the degree of need for attention > are more like this: > > error (failed tests - you should look into it) > skip (skipped - perhaps you need more packages?) > warn (expected failure - you may want to look into fixing it someday) > info > pass > > The "expected_failure" cases painted in "warn" are all long-known > failures; I do not think reminding about them in "bold" over and > over will help encouraging the developers take a look at them. As Peff already noted, on many (most?) X terminals "bold" colours are just brighter colours, rather than a heavier typeface. How bold they look is therefore dependent on the colour scheme used by that terminal. > The "skipped" cases fall into two categories. Either you already > know you choose to not to care (e.g. I do not expect to use git-p4 > and decided not to install p4 anywhere, so I may have t98?? on > GIT_SKIP_TESTS environment) or you haven't reached that point on a > new system and haven't realized that you didn't install a package > needed to run tests you care about (e.g. cvsserver tests would not > run without Perl interface to SQLite). For the former, the bold > output is merely distracting; for the latter, bold _might_ help in > this case. Very good point. > At least, I think > > GIT_SKIP_TESTS=t98?? sh t9800-git-p4-basic.sh -v > > should paint "skipping test t9800 altogether" (emitted with "-v) and > the last line "1..0 # SKIP skip all tests in t9800" both in the same > "info" color. > > How about going further to reduce "bold" a bit more, like this? > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index aaf013e..2bbb81d 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -182,13 +182,13 @@ then > error) > tput bold; tput setaf 1;; # bold red > skip) > - tput bold; tput setaf 4;; # bold blue > + tput setaf 4;; # bold blue The comment still says "bold". > warn) > - tput bold; tput setaf 3;; # bold brown/yellow > + tput setaf 3;; # bold brown/yellow Ditto here ... > pass) > tput setaf 2;;# green > info) > - tput bold; tput setaf 6;; # bold cyan > + tput setaf 6;; # bold cyan ... and here. > *) > test -n "$quiet" && return;; > esac > @@ -589,7 +589,7 @@ for skp in $GIT_SKIP_TESTS > do > case "$this_test" in > $skp) > - say_color skip >&3 "skipping test $this_test altogether" > + say_color info >&3 "skipping test $this_test altogether" > skip_all="skip all tests in $this_test" > test_done > esac Yes, I like this last hunk especially. I have no objection in principle to a reduction in boldness. However, I am beginning to get disheartened that at this rate, this series will never land. I already submitted v4 of the series which already had non-bold blue. I then received feedback indicating that bold blue would be more suitable, so despite alarm bells beginning to ring in my head, I submitted v5 with bold blue, declaring that that would be my last version: http://article.gmane.org/gmane.comp.version-control.git/206042 A further concern about "info" messages not being blue prompted me to attempt to canvass more opinions: http://article.gmane.org/gmane.comp.version-control.git/209321 I received none, so submitted v6 based on my best judgement. Now we are talking about a potential v7 going *back* to non-bold blue. I can submit v7 if you think it's worth it, but would that really be the end of the discussion? It's clear from the above that colour scheme design by committee is about as good an idea as asking a bunch of kids to reach consensus on their favourite colour ;-) So if possible I'd be very happy for Junio to simply make an executive decision (I don't care which way, as long as it fits the traffic lights scheme and uses distinct
Re: [PATCH v6 0/7] make test output coloring more intuitive
On Thu, Dec 20, 2012 at 11:21:09AM -0800, Junio C Hamano wrote: > The "expected_failure" cases painted in "warn" are all long-known > failures; I do not think reminding about them in "bold" over and > over will help encouraging the developers take a look at them. > > The "skipped" cases fall into two categories. Either you already > know you choose to not to care (e.g. I do not expect to use git-p4 > and decided not to install p4 anywhere, so I may have t98?? on > GIT_SKIP_TESTS environment) or you haven't reached that point on a > new system and haven't realized that you didn't install a package > needed to run tests you care about (e.g. cvsserver tests would not > run without Perl interface to SQLite). For the former, the bold > output is merely distracting; for the latter, bold _might_ help in > this case. > > At least, I think > > GIT_SKIP_TESTS=t98?? sh t9800-git-p4-basic.sh -v > > should paint "skipping test t9800 altogether" (emitted with "-v) and > the last line "1..0 # SKIP skip all tests in t9800" both in the same > "info" color. > > How about going further to reduce "bold" a bit more, like this? Yeah, I think it is a little easier on the eyes while maintaining the intended color scheme. > diff --git a/t/test-lib.sh b/t/test-lib.sh > index aaf013e..2bbb81d 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -182,13 +182,13 @@ then > error) > tput bold; tput setaf 1;; # bold red > skip) > - tput bold; tput setaf 4;; # bold blue > + tput setaf 4;; # bold blue On my xterm, at least, this is actually the difference between light blue" and dark blue, not bold and not-bold. I think it is OK, though to be honest, having seen the "skip all" messages in cyan (e.g., running t9800), I think just printing skip messages in cyan looks best. But it is not that big a deal to me, and we are well into bikeshed territory, I think, so that will be my last word on the subject. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] make test output coloring more intuitive
Jeff King writes: >> Good point, I forgot to check what it looked like with -v. Since this >> series is already on v6, is there a more lightweight way of addressing >> this tiny tweak than sending v7? > > It is ultimately up to Junio, but I suspect he would be OK if you just > reposted patch 4/7 with the above squashed. Or even just said "I like > this, please squash it into patch 4 (change info messages from > yellow/brown to bold cyan). Surely; as long as the series is not in 'next', the change to be squashed is not too big and it is not too much work (and in this case it certainly is not). I actually wonder if "skipped test in bold blue" and "known breakage in bold yellow" should also lose the boldness. Errors and warnings in bold are good, but I would say the degree of need for attention are more like this: error (failed tests - you should look into it) skip (skipped - perhaps you need more packages?) warn (expected failure - you may want to look into fixing it someday) info pass The "expected_failure" cases painted in "warn" are all long-known failures; I do not think reminding about them in "bold" over and over will help encouraging the developers take a look at them. The "skipped" cases fall into two categories. Either you already know you choose to not to care (e.g. I do not expect to use git-p4 and decided not to install p4 anywhere, so I may have t98?? on GIT_SKIP_TESTS environment) or you haven't reached that point on a new system and haven't realized that you didn't install a package needed to run tests you care about (e.g. cvsserver tests would not run without Perl interface to SQLite). For the former, the bold output is merely distracting; for the latter, bold _might_ help in this case. At least, I think GIT_SKIP_TESTS=t98?? sh t9800-git-p4-basic.sh -v should paint "skipping test t9800 altogether" (emitted with "-v) and the last line "1..0 # SKIP skip all tests in t9800" both in the same "info" color. How about going further to reduce "bold" a bit more, like this? diff --git a/t/test-lib.sh b/t/test-lib.sh index aaf013e..2bbb81d 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -182,13 +182,13 @@ then error) tput bold; tput setaf 1;; # bold red skip) - tput bold; tput setaf 4;; # bold blue + tput setaf 4;; # bold blue warn) - tput bold; tput setaf 3;; # bold brown/yellow + tput setaf 3;; # bold brown/yellow pass) tput setaf 2;;# green info) - tput bold; tput setaf 6;; # bold cyan + tput setaf 6;; # bold cyan *) test -n "$quiet" && return;; esac @@ -589,7 +589,7 @@ for skp in $GIT_SKIP_TESTS do case "$this_test" in $skp) - say_color skip >&3 "skipping test $this_test altogether" + say_color info >&3 "skipping test $this_test altogether" skip_all="skip all tests in $this_test" test_done esac -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] make test output coloring more intuitive
On Thu, Dec 20, 2012 at 4:11 PM, Jeff King wrote: > On Thu, Dec 20, 2012 at 03:44:53PM +, Adam Spiers wrote: >> > diff --git a/t/test-lib.sh b/t/test-lib.sh >> > index 256f1c6..31f59af 100644 >> > --- a/t/test-lib.sh >> > +++ b/t/test-lib.sh >> > @@ -227,7 +227,7 @@ then >> > pass) >> > tput setaf 2;;# green >> > info) >> > - tput bold; tput setaf 6;; # bold cyan >> > + tput setaf 6;; # cyan >> > *) >> > test -n "$quiet" && return;; >> > esac >> > >> >> Good point, I forgot to check what it looked like with -v. Since this >> series is already on v6, is there a more lightweight way of addressing >> this tiny tweak than sending v7? > > It is ultimately up to Junio, but I suspect he would be OK if you just > reposted patch 4/7 with the above squashed. I'll do that if Junio is OK with that. > Or even just said "I like > this, please squash it into patch 4 (change info messages from > yellow/brown to bold cyan). Yes, I'm OK with this way too :) Of course "bold" would need to be dropped from the commit message. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] make test output coloring more intuitive
On Thu, Dec 20, 2012 at 03:44:53PM +, Adam Spiers wrote:
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 256f1c6..31f59af 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -227,7 +227,7 @@ then
> > pass)
> > tput setaf 2;;# green
> > info)
> > - tput bold; tput setaf 6;; # bold cyan
> > + tput setaf 6;; # cyan
> > *)
> > test -n "$quiet" && return;;
> > esac
> >
>
> Good point, I forgot to check what it looked like with -v. Since this
> series is already on v6, is there a more lightweight way of addressing
> this tiny tweak than sending v7?
It is ultimately up to Junio, but I suspect he would be OK if you just
reposted patch 4/7 with the above squashed. Or even just said "I like
this, please squash it into patch 4 (change info messages from
yellow/brown to bold cyan).
As an aside, it made me wonder how hard/useful it would be to color the
snippets even more. Doing this:
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index f9ccbf2..3d44a94 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -364,7 +364,12 @@ test_expect_success () {
export test_prereq
if ! test_skip "$@"
then
- say >&3 "expecting success: $2"
+ if test -z "$GIT_TEST_HIGHLIGHT"; then
+ say >&3 "expecting success: $2"
+ else
+ say >&3 "expecting success:"
+ echo "$2" | eval "$GIT_TEST_HIGHLIGHT"
+ fi
if test_run_ "$2"
then
test_ok_ "$1"
produces highlighted snippets with:
GIT_TEST_HIGHLIGHT='highlight -S sh -O ansi'
or
GIT_TEST_HIGHLIGHT='pygmentize -l sh'
depending on what you have installed on your system. I'm not convinced
it actually adds anything, but it's a fun toy. A real patch would
probably turn it off in non-verbose mode, as invoking the highlighter
(especially pygmentize) repeatedly is somewhat expensive.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] make test output coloring more intuitive
On Thu, Dec 20, 2012 at 3:34 PM, Jeff King wrote: > On Sun, Dec 16, 2012 at 07:01:56PM +, Adam Spiers wrote: >> On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano wrote: >> > Adam Spiers writes: >> >> This series of commits attempts to make test output coloring >> >> more intuitive,... >> > >> > Thanks; I understand that this is to replace the previous one >> > b465316 (tests: paint unexpectedly fixed known breakages in bold >> > red, 2012-09-19)---am I correct? >> >> Correct. AFAICS I have incorporated all feedback raised in previous >> reviews. >> >> > Will take a look; thanks. >> >> Thanks. Sorry again for the delay. I'm now (finally) resuming work >> on as/check-ignore. > > I eyeballed the test output of "pu". I do think this resolves all of the > issues brought up before, and I really hate to bikeshed on the colors at > this point, but I find that bold cyan a bit hard on the eyes when > running with "-v" (where most of the output is in that color, as it > dumps the shell for each test). Is there any reason not to tone it down > a bit like: > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 256f1c6..31f59af 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -227,7 +227,7 @@ then > pass) > tput setaf 2;;# green > info) > - tput bold; tput setaf 6;; # bold cyan > + tput setaf 6;; # cyan > *) > test -n "$quiet" && return;; > esac > > -Peff Good point, I forgot to check what it looked like with -v. Since this series is already on v6, is there a more lightweight way of addressing this tiny tweak than sending v7? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] make test output coloring more intuitive
On Sun, Dec 16, 2012 at 07:01:56PM +, Adam Spiers wrote: > On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano wrote: > > Adam Spiers writes: > > > >> This series of commits attempts to make test output coloring > >> more intuitive,... > > > > Thanks; I understand that this is to replace the previous one > > b465316 (tests: paint unexpectedly fixed known breakages in bold > > red, 2012-09-19)---am I correct? > > Correct. AFAICS I have incorporated all feedback raised in previous > reviews. > > > Will take a look; thanks. > > Thanks. Sorry again for the delay. I'm now (finally) resuming work > on as/check-ignore. I eyeballed the test output of "pu". I do think this resolves all of the issues brought up before, and I really hate to bikeshed on the colors at this point, but I find that bold cyan a bit hard on the eyes when running with "-v" (where most of the output is in that color, as it dumps the shell for each test). Is there any reason not to tone it down a bit like: diff --git a/t/test-lib.sh b/t/test-lib.sh index 256f1c6..31f59af 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -227,7 +227,7 @@ then pass) tput setaf 2;;# green info) - tput bold; tput setaf 6;; # bold cyan + tput setaf 6;; # cyan *) test -n "$quiet" && return;; esac -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] make test output coloring more intuitive
Adam Spiers writes: > On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano wrote: >> Adam Spiers writes: >> >>> This series of commits attempts to make test output coloring >>> more intuitive,... >> >> Thanks; I understand that this is to replace the previous one >> b465316 (tests: paint unexpectedly fixed known breakages in bold >> red, 2012-09-19)---am I correct? > > Correct. AFAICS I have incorporated all feedback raised in previous > reviews. Seemed clean from a cursory look. Will replace. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] make test output coloring more intuitive
On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano wrote: > Adam Spiers writes: > >> This series of commits attempts to make test output coloring >> more intuitive,... > > Thanks; I understand that this is to replace the previous one > b465316 (tests: paint unexpectedly fixed known breakages in bold > red, 2012-09-19)---am I correct? Correct. AFAICS I have incorporated all feedback raised in previous reviews. > Will take a look; thanks. Thanks. Sorry again for the delay. I'm now (finally) resuming work on as/check-ignore. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] make test output coloring more intuitive
Adam Spiers writes: > This series of commits attempts to make test output coloring > more intuitive,... Thanks; I understand that this is to replace the previous one b465316 (tests: paint unexpectedly fixed known breakages in bold red, 2012-09-19)---am I correct? > - red is only used for things which have gone unexpectedly wrong: > test failures, unexpected test passes, and failures with the > framework, > > - yellow is only used for known breakages, > > - green is only used for things which have gone to plan and > require no further work to be done, > > - blue is only used for skipped tests, and > > - cyan is used for other informational messages. OK. > Since unexpected test passes are no longer treated as passes, the > summary lines displayed at the end of a test run have enough different > possible outputs to warrant them being covered in the test framework's > self-tests. Therefore this series also refactors and extends the > self-tests. > > Adam Spiers (7): > tests: test number comes first in 'not ok $count - $message' > tests: paint known breakages in bold yellow > tests: paint skipped tests in bold blue > tests: change info messages from yellow/brown to bold cyan > tests: refactor mechanics of testing in a sub test-lib > tests: test the test framework more thoroughly > tests: paint unexpectedly fixed known breakages in bold red > > t/t-basic.sh | 211 > ++- > t/test-lib.sh| 25 --- > 2 files changed, 180 insertions(+), 56 deletions(-) Will take a look; thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/7] make test output coloring more intuitive
This series of commits attempts to make test output coloring more intuitive, so that: - red is only used for things which have gone unexpectedly wrong: test failures, unexpected test passes, and failures with the framework, - yellow is only used for known breakages, - green is only used for things which have gone to plan and require no further work to be done, - blue is only used for skipped tests, and - cyan is used for other informational messages. Since unexpected test passes are no longer treated as passes, the summary lines displayed at the end of a test run have enough different possible outputs to warrant them being covered in the test framework's self-tests. Therefore this series also refactors and extends the self-tests. Adam Spiers (7): tests: test number comes first in 'not ok $count - $message' tests: paint known breakages in bold yellow tests: paint skipped tests in bold blue tests: change info messages from yellow/brown to bold cyan tests: refactor mechanics of testing in a sub test-lib tests: test the test framework more thoroughly tests: paint unexpectedly fixed known breakages in bold red t/t-basic.sh | 211 ++- t/test-lib.sh| 25 --- 2 files changed, 180 insertions(+), 56 deletions(-) -- 1.7.12.1.396.g53b3ea9 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] fix 'make test' for HP NonStop
On Tue, Oct 30, 2012 at 10:21:40AM +0100, Joachim Schmitz wrote: > This fixes the vast majority of test failures on HP NonStop. > Some test don't work with /bin/diff, some fail with /bin/tar, > so let's put /usr/local/bin in PATH first. > Some tests fail with /bin/sh (link to /bin/ksh) so use bash instead > > Signed-off-by: Joachim Schmitz > --- > > Makefile | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/Makefile b/Makefile > index f69979e..35380dd 100644 > --- a/Makefile > +++ b/Makefile > @@ -1381,6 +1381,10 @@ ifeq ($(uname_S),NONSTOP_KERNEL) > MKDIR_WO_TRAILING_SLASH = YesPlease > # RFE 10-120912-4693 submitted to HP NonStop development. > NO_SETITIMER = UnfortunatelyYes > + SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin > + SHELL_PATH=/usr/local/bin/bash > + # as of H06.25/J06.14, we might better use this > + #SHELL_PATH=/usr/coreutils/bin/bash > endif > ifneq (,$(findstring MINGW,$(uname_S))) > pathsep = ; Your patch was whitespace damaged, but I was able to fix it up. Thanks. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] fix 'make test' for HP NonStop
> From: Jeff King [mailto:p...@peff.net] > Sent: Monday, October 29, 2012 8:07 AM > To: Joachim Schmitz > Cc: git@vger.kernel.org > Subject: Re: [PATCH v2] fix 'make test' for HP NonStop > > On Thu, Oct 25, 2012 at 12:57:10PM +0200, Joachim Schmitz wrote: > > > diff --git a/Makefile b/Makefile > > index f69979e..35380dd 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1381,6 +1381,15 @@ ifeq ($(uname_S),NONSTOP_KERNEL) > > MKDIR_WO_TRAILING_SLASH = YesPlease > > # RFE 10-120912-4693 submitted to HP NonStop development. > > NO_SETITIMER = UnfortunatelyYes > > + > > + # for 'make test' > > + # some test don't work with /bin/diff, some fail with /bin/tar > > + # some need bash, and some need /usr/local/bin in PATH first > > + SHELL_PATH=/usr/local/bin/bash > > + SANE_TOOL_PATH=/usr/local/bin > > I think we can drop these comments, as the reasoning really should just > go in the commit message. OK by me. > > + # as of H06.25/J06.14, we might better use this > > + #SHELL_PATH=/usr/coreutils/bin/bash > > + #SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin > > Is there any reason not to put both into the default SANE_TOOL_PATH? If > /usr/coreutils/bin does not exist on older versions, it will be a > harmless no-op. I guess we arestuck with picking one $SHELL_PATH, > though. And because of that have to modify something anyway... But I don't really mind about an extended SANE_TOOL_PATH > -Peff Bye, Jojo -- 8< -- This fixes the vast majority of test failures on HP NonStop. Some test don't work with /bin/diff, some fail with /bin/tar, so let's put /usr/local/bin in PATH first. Some tests fail with /bin/sh (link to /bin/ksh) so use bash instead Signed-off-by: Joachim Schmitz --- Makefile | 9 + 1 file changed, 9 insertions(+) diff --git a/Makefile b/Makefile index f69979e..35380dd 100644 --- a/Makefile +++ b/Makefile @@ -1381,6 +1381,10 @@ ifeq ($(uname_S),NONSTOP_KERNEL) MKDIR_WO_TRAILING_SLASH = YesPlease # RFE 10-120912-4693 submitted to HP NonStop development. NO_SETITIMER = UnfortunatelyYes + SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin + SHELL_PATH=/usr/local/bin/bash + # as of H06.25/J06.14, we might better use this + #SHELL_PATH=/usr/coreutils/bin/bash endif ifneq (,$(findstring MINGW,$(uname_S))) pathsep = ; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] fix 'make test' for HP NonStop
On Thu, Oct 25, 2012 at 12:57:10PM +0200, Joachim Schmitz wrote: > diff --git a/Makefile b/Makefile > index f69979e..35380dd 100644 > --- a/Makefile > +++ b/Makefile > @@ -1381,6 +1381,15 @@ ifeq ($(uname_S),NONSTOP_KERNEL) > MKDIR_WO_TRAILING_SLASH = YesPlease > # RFE 10-120912-4693 submitted to HP NonStop development. > NO_SETITIMER = UnfortunatelyYes > + > + # for 'make test' > + # some test don't work with /bin/diff, some fail with /bin/tar > + # some need bash, and some need /usr/local/bin in PATH first > + SHELL_PATH=/usr/local/bin/bash > + SANE_TOOL_PATH=/usr/local/bin I think we can drop these comments, as the reasoning really should just go in the commit message. > + # as of H06.25/J06.14, we might better use this > + #SHELL_PATH=/usr/coreutils/bin/bash > + #SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin Is there any reason not to put both into the default SANE_TOOL_PATH? If /usr/coreutils/bin does not exist on older versions, it will be a harmless no-op. I guess we arestuck with picking one $SHELL_PATH, though. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] fix 'make test' for HP NonStop
> From: Jeff King [mailto:p...@peff.net] > Sent: Thursday, October 25, 2012 12:53 PM > To: Joachim Schmitz > Cc: git@vger.kernel.org > Subject: Re: [PATCH] fix 'make test' for HP NonStop > > On Thu, Oct 25, 2012 at 12:51:59PM +0200, Joachim Schmitz wrote: > > > > But then I would think using /usr/local would be the sane thing to put > > > there, if that is the closest to "standard" for your platform. > > > > OK, yes, hardcoding /usr/local seems OK too. > > Would I need to re-roll? > > Please do. Done. For some reason not 'chained' to this thread though Bye, Jojo -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] fix 'make test' for HP NonStop
This fixes the vast majority of test failures on HP NonStop.
Signed-off-by: Joachim Schmitz
---
v2: hardcode /usr/local rather than using ${prefix}
Makefile | 9 +
1 file changed, 9 insertions(+)
diff --git a/Makefile b/Makefile
index f69979e..35380dd 100644
--- a/Makefile
+++ b/Makefile
@@ -1381,6 +1381,15 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
MKDIR_WO_TRAILING_SLASH = YesPlease
# RFE 10-120912-4693 submitted to HP NonStop development.
NO_SETITIMER = UnfortunatelyYes
+
+ # for 'make test'
+ # some test don't work with /bin/diff, some fail with /bin/tar
+ # some need bash, and some need /usr/local/bin in PATH first
+ SHELL_PATH=/usr/local/bin/bash
+ SANE_TOOL_PATH=/usr/local/bin
+ # as of H06.25/J06.14, we might better use this
+ #SHELL_PATH=/usr/coreutils/bin/bash
+ #SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin
endif
ifneq (,$(findstring MINGW,$(uname_S)))
pathsep = ;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix 'make test' for HP NonStop
On Thu, Oct 25, 2012 at 12:51:59PM +0200, Joachim Schmitz wrote: > > But then I would think using /usr/local would be the sane thing to put > > there, if that is the closest to "standard" for your platform. > > OK, yes, hardcoding /usr/local seems OK too. > Would I need to re-roll? Please do. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] fix 'make test' for HP NonStop
> From: Jeff King [mailto:p...@peff.net]
> Sent: Thursday, October 25, 2012 12:49 PM
> To: Joachim Schmitz
> Cc: git@vger.kernel.org
> Subject: Re: [PATCH] fix 'make test' for HP NonStop
>
> On Thu, Oct 25, 2012 at 12:21:44PM +0200, Joachim Schmitz wrote:
>
> > > > + # for 'make test'
> > > > + # some test don't work with /bin/diff, some fail with /bin/tar
> > > > + # some need bash, and some need ${prefix}/bin in PATH first
> > > > + SHELL_PATH=${prefix}/bin/bash
> > > > + SANE_TOOL_PATH=${prefix}/bin
> > >
> > > This feels a little too specific to go in our Makefile. Do we have any
> > > reason to think that where you are installing git is going to be the
> > > same place you have bash and other sane tools? Wouldn't this mean that
> > > things work when you run "make" but mysteriously break when you run
> > > "make prefix=/my/local/install/of/git"?
> >
> > Well, "make" won't break (I think), but "make test" very well might.
>
> Sure.
>
> > Well, so far all OpenSource packages ported to HP NonStop (at least
> > the ones on ituglib.connect-community.org) use prefix=/usr/local and
> > there is no intention to change that.
>
> But then I would think using /usr/local would be the sane thing to put
> there, if that is the closest to "standard" for your platform.
OK, yes, hardcoding /usr/local seems OK too.
Would I need to re-roll?
Bye, Jojo
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix 'make test' for HP NonStop
On Thu, Oct 25, 2012 at 12:21:44PM +0200, Joachim Schmitz wrote:
> > > + # for 'make test'
> > > + # some test don't work with /bin/diff, some fail with /bin/tar
> > > + # some need bash, and some need ${prefix}/bin in PATH first
> > > + SHELL_PATH=${prefix}/bin/bash
> > > + SANE_TOOL_PATH=${prefix}/bin
> >
> > This feels a little too specific to go in our Makefile. Do we have any
> > reason to think that where you are installing git is going to be the
> > same place you have bash and other sane tools? Wouldn't this mean that
> > things work when you run "make" but mysteriously break when you run
> > "make prefix=/my/local/install/of/git"?
>
> Well, "make" won't break (I think), but "make test" very well might.
Sure.
> Well, so far all OpenSource packages ported to HP NonStop (at least
> the ones on ituglib.connect-community.org) use prefix=/usr/local and
> there is no intention to change that.
But then I would think using /usr/local would be the sane thing to put
there, if that is the closest to "standard" for your platform.
If there is not a standard, then I think we are better off leaving it
blank and letting people do the right thing for their system (including
packagers who are building for other people).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] fix 'make test' for HP NonStop
> From: Jeff King [mailto:p...@peff.net]
> Sent: Thursday, October 25, 2012 11:58 AM
> To: Joachim Schmitz
> Cc: git@vger.kernel.org
> Subject: Re: [PATCH] fix 'make test' for HP NonStop
>
> On Mon, Oct 22, 2012 at 04:30:17PM +0200, Joachim Schmitz wrote:
>
> > + # for 'make test'
> > + # some test don't work with /bin/diff, some fail with /bin/tar
> > + # some need bash, and some need ${prefix}/bin in PATH first
> > + SHELL_PATH=${prefix}/bin/bash
> > + SANE_TOOL_PATH=${prefix}/bin
>
> This feels a little too specific to go in our Makefile. Do we have any
> reason to think that where you are installing git is going to be the
> same place you have bash and other sane tools? Wouldn't this mean that
> things work when you run "make" but mysteriously break when you run
> "make prefix=/my/local/install/of/git"?
Well, "make" won't break (I think), but "make test" very well might.
Well, so far all OpenSource packages ported to HP NonStop (at least the ones on
ituglib.connect-community.org) use prefix=/usr/local and there is no intention
to change that.
A few (bash, vim, coreutils, tar, gzip, bzip2) get delivered with the system
meanwhile (rather than having to be downloaded and installed by the customer)
and live in /usr/corutiles. Still more are needed (e.g. diff , make).
The next lines in my patch cater for that, it is missing though an automatic
switch.
Such a switch would be possible, using 'uname -r' and 'uname -v', but pretty
convoluted
Pseudo code:
If (`uname -r` = J06 && `uname -v` >= 14) || (`uname -r`= H06 && `uname -v` >=
25)
SHELL_PATH=/usr/coreutils/bin/bash
SANE_TOOL_PATH=/usr/coreutils/bin:${prefix}/bin
else
SHELL_PATH=${prefix}/bin/bash
SANE_TOOL_PATH=${prefix}/bin
endif
I didn't deem it worth the effort. As mentioned it'd most likely still need
stuff from /usr/local/bin
And someone wanting prefix somewhere else could still do
PATH=/usr/local/bin:$PATH make prefix=/my/local/install/of/git
And so find what's need in either the 1st or 2nd path of PATH
Bye, Jojo
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fix 'make test' for HP NonStop
On Mon, Oct 22, 2012 at 04:30:17PM +0200, Joachim Schmitz wrote:
> + # for 'make test'
> + # some test don't work with /bin/diff, some fail with /bin/tar
> + # some need bash, and some need ${prefix}/bin in PATH first
> + SHELL_PATH=${prefix}/bin/bash
> + SANE_TOOL_PATH=${prefix}/bin
This feels a little too specific to go in our Makefile. Do we have any
reason to think that where you are installing git is going to be the
same place you have bash and other sane tools? Wouldn't this mean that
things work when you run "make" but mysteriously break when you run
"make prefix=/my/local/install/of/git"?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fix 'make test' for HP NonStop
This fixes the vast majority of test failures on HP NonStop.
Signed-off-by: Joachim Schmitz
---
A few more still insist on /usr/local/bin being 1st in PATH and having done that
we're down to one single failing test, t0301 #12 "helper (cache --timeout=1)
times out"
Makefile | 9 +
1 file changed, 9 insertions(+)
diff --git a/Makefile b/Makefile
index f69979e..35380dd 100644
--- a/Makefile
+++ b/Makefile
@@ -1381,6 +1381,15 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
MKDIR_WO_TRAILING_SLASH = YesPlease
# RFE 10-120912-4693 submitted to HP NonStop development.
NO_SETITIMER = UnfortunatelyYes
+
+ # for 'make test'
+ # some test don't work with /bin/diff, some fail with /bin/tar
+ # some need bash, and some need ${prefix}/bin in PATH first
+ SHELL_PATH=${prefix}/bin/bash
+ SANE_TOOL_PATH=${prefix}/bin
+ # as of H06.25/J06.14, we might better use this
+ #SHELL_PATH=/usr/coreutils/bin/bash
+ #SANE_TOOL_PATH=/usr/coreutils/bin:${prefix}/bin
endif
ifneq (,$(findstring MINGW,$(uname_S)))
pathsep = ;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make test
"Joachim Schmitz" schrieb im Newsbeitrag
news:...
> Hi folks
>
> I'm trying to understand why certain tests in 'make test' fail. Here's the
> first one
>
> $ ../git --version
> git version 1.8.0.rc2.5.g6b89306
> $ GIT_TEST_CMP_USE_COPIED_CONTEXT=true ./t-basic.sh # our diff doesn't
> understand -u
> ok 1 - .git/objects should be empty after git init in an empty repo
> ...
> ok 3 - success is reported like this
> not ok 4 - pretend we have a known breakage # TODO known breakage
>
> This is expected, right?
>
> ok 5 - pretend we have fixed a known breakage (run in sub test-lib)
> ...
> ok 11 - tests clean up after themselves
>
> the next is not though? Why might it be failing, where to check?
>
> not ok - 12 tests clean up even on failures
> #
> # mkdir failing-cleanup &&
> # (
> # cd failing-cleanup &&
> #
> # cat >failing-cleanup.sh <<-EOF &&
> # #!/bin/sh
> #
> # test_description='Failing tests with cleanup commands'
> #
> # # Point to the t/test-lib.sh, which isn't in ../ as usual
> # TEST_DIRECTORY="/home/jojo/git/git/t"
> # . "$TEST_DIRECTORY"/test-lib.sh
> #
> # test_expect_success 'tests clean up even after a failure' '
> # touch clean-after-failure &&
> # test_when_finished rm clean-after-failure &&
> # (exit 1)
> # '
> # test_expect_success 'failure to clean up causes the test to
> fail' '
> # test_when_finished "(exit 2)"
> # '
> # test_done
> #
> # EOF
> #
> # chmod +x failing-cleanup.sh &&
> # test_must_fail ./failing-cleanup.sh >out 2>err &&
> # ! test -s err &&
> # ! test -f "trash
> directory.failing-cleanup/clean-after-failure" &&
> # sed -e 's/Z$//' -e 's/^> //' >expect <<-\EOF &&
> # > not ok - 1 tests clean up even after a failure
> # > # Z
> # > # touch clean-after-failure &&
> # > # test_when_finished rm clean-after-failure &&
> # > # (exit 1)
> # > # Z
> # > not ok - 2 failure to clean up causes the test to fail
> # > # Z
> # > # test_when_finished "(exit 2)"
> # > # Z
> # > # failed 2 among 2 test(s)
> # > 1..2
> # EOF
> # test_cmp expect out
> # )
> #
> ok 13 - git update-index without --add should fail adding
> ...
> ok 47 - very long name in the index handled sanely
> # still have 1 known breakage(s)
> # failed 1 among remaining 46 test(s)
> 1..47
As mentioned elsethread this works if using bash rather than the system's sh
(which is a ksh)
But there are several other failures. After some investigations and experiments
I found the following tests to fail with the system
provided grep (for which I had to set GIT_TEST_CMP_USE_COPIED_CONTEXT), but
succeed with GNU grep:
t3308 #14, #15, #17and #19
t3310 #10, #12, #14 and #18
t4047 #38 and #39
t4050 #2 and #3
t4116 #3, #4 and #5
t5509 #2
t7401 #18
The following fail with the system provided tar, but succeed with GNU tar:
t0024 #2
t4116 #4,
t5000 #14, #16, #20, #24, #26 and #51
t5001 #2, #6, #10 and #15
The following tests fail with the system provided sh (which is a ksh really),
but succeed with bash:
t #12
t0001 #20
t1450 #17 and #18
(t0204 #3 and #8 succeed in sh but fail in bash. They succeed in bash too when
/usr/local/bin is in PATH first though, which would
sort the diff and tar problem too, need to investigate why)
t3006 #2 and #3
t3403 #4, #5, #8 and #9
t3404 #2 - #13, #14 - #18, #20 - #41, #44, #46 - #70
t3409 #2 - #5
t3410 #2 and #3
t3411 #2 and #3
t3412 #8, #10 - #12, #15, #17, #23, #25, -26, #28, #29, #31
t3413 #3, #5 - #10, #14, #15
and many more...
The following needs bash and /usr/local/bin first in PATH
("PATH=/usr/local/bin:$PATH make test")
t0204 #3 and #8 (or just sh, see above)
t3032 #11
t3900 #24, #25
t4201 #8
t5000 #14
t5150 #6
I though "SANE_TOOL_PATH=/usr/local/bin" plus "SHELL_PATH=/usr/local/bin/bash"
would to fix the all but it does not and instead
brings up some other failures too:
t5521 #2
RE: make test
> From: Joachim Schmitz [mailto:j...@schmitz-digital.de] > Sent: Monday, October 15, 2012 3:18 PM > To: 'Andreas Schwab'; 'Johannes Sixt' > Cc: 'git@vger.kernel.org' > Subject: RE: make test > > > From: Andreas Schwab [mailto:sch...@linux-m68k.org] > > Sent: Monday, October 15, 2012 2:35 PM > > To: Johannes Sixt > > Cc: Joachim Schmitz; git@vger.kernel.org > > Subject: Re: make test > > > > Johannes Sixt writes: > > > > > Am 10/15/2012 13:58, schrieb Joachim Schmitz: > > >> ++ mkdir failing-cleanup > > >> ++ cd failing-cleanup > > >> ++ cat > > >> ++ chmod +x failing-cleanup.sh > > >> ++ test_must_fail ./failing-cleanup.sh > > >> + eval_ret=1 > > > > > > I wonder why the log does not show the commands of function > > > test_must_fail. > > > > That's because stderr is redirected. > > > > cat err > ++ ./failing-cleanup.sh > ++ exit_code=0 > ++ test 0 = 0 > ++ echo 'test_must_fail: command succeeded: ./failing-cleanup.sh' > test_must_fail: command succeeded: ./failing-cleanup.sh > ++ return 1 That test (as well as quite a few more) do pass when using bash rather than our sh (which really is a ksh) Bye, Jojo -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: make test
> From: Andreas Schwab [mailto:sch...@linux-m68k.org] > Sent: Monday, October 15, 2012 2:35 PM > To: Johannes Sixt > Cc: Joachim Schmitz; git@vger.kernel.org > Subject: Re: make test > > Johannes Sixt writes: > > > Am 10/15/2012 13:58, schrieb Joachim Schmitz: > >> ++ mkdir failing-cleanup > >> ++ cd failing-cleanup > >> ++ cat > >> ++ chmod +x failing-cleanup.sh > >> ++ test_must_fail ./failing-cleanup.sh > >> + eval_ret=1 > > > > I wonder why the log does not show the commands of function > > test_must_fail. > > That's because stderr is redirected. > cat err ++ ./failing-cleanup.sh ++ exit_code=0 ++ test 0 = 0 ++ echo 'test_must_fail: command succeeded: ./failing-cleanup.sh' test_must_fail: command succeeded: ./failing-cleanup.sh ++ return 1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: make test
> From: Johannes Sixt [mailto:j.s...@viscovery.net] > Sent: Monday, October 15, 2012 2:10 PM > To: Joachim Schmitz > Cc: git@vger.kernel.org > Subject: Re: make test > > Am 10/15/2012 13:58, schrieb Joachim Schmitz: > > ++ mkdir failing-cleanup > > ++ cd failing-cleanup > > ++ cat > > ++ chmod +x failing-cleanup.sh > > ++ test_must_fail ./failing-cleanup.sh > > + eval_ret=1 > > I wonder why the log does not show the commands of function > test_must_fail. Is there a 'set +x' hidden somewhere? Run > ./failing-cleanup.sh manually. Check its exit code (it should be non-zero, > but not something strange like 127 and above; see test_must_fail()) and > dig further from there. I'll stop here. It returns 0. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make test
Johannes Sixt writes: > Am 10/15/2012 13:58, schrieb Joachim Schmitz: >> ++ mkdir failing-cleanup >> ++ cd failing-cleanup >> ++ cat >> ++ chmod +x failing-cleanup.sh >> ++ test_must_fail ./failing-cleanup.sh >> + eval_ret=1 > > I wonder why the log does not show the commands of function > test_must_fail. That's because stderr is redirected. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make test
Am 10/15/2012 13:58, schrieb Joachim Schmitz: > ++ mkdir failing-cleanup > ++ cd failing-cleanup > ++ cat > ++ chmod +x failing-cleanup.sh > ++ test_must_fail ./failing-cleanup.sh > + eval_ret=1 I wonder why the log does not show the commands of function test_must_fail. Is there a 'set +x' hidden somewhere? Run ./failing-cleanup.sh manually. Check its exit code (it should be non-zero, but not something strange like 127 and above; see test_must_fail()) and dig further from there. I'll stop here. -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: make test
> -Original Message- > From: Johannes Sixt [mailto:j.s...@viscovery.net] > Sent: Monday, October 15, 2012 1:53 PM > To: Joachim Schmitz > Cc: git@vger.kernel.org > Subject: Re: make test > > Am 10/15/2012 13:37, schrieb Joachim Schmitz: > > ... > > + eval ' > > find .git/objects -type f -print >should-be-empty && > > test_line_count = 0 should-be-empty > > ' > > ++ find .git/objects -type f -print > > ++ test_line_count = 0 should-be-empty > > ++ test 3 '!=' 3 > > +++ wc -l > > ++ test 0 = 0 > > + eval_ret=0 > > This is the key line. If it is 'eval_ret=1' (or other non-zero value), > then the test failed, and the lines above it usually indicate where in the > test snippet the failure occurred. ... ++ mkdir failing-cleanup ++ cd failing-cleanup ++ cat ++ chmod +x failing-cleanup.sh ++ test_must_fail ./failing-cleanup.sh + eval_ret=1 + test -z t + test 1 = 0 + test -n '' + test t = t + test -n '' + return 1 + test_failure_ 'tests clean up even on failures' ' ... This part? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make test
Am 10/15/2012 13:37, schrieb Joachim Schmitz: > ... > + eval ' > find .git/objects -type f -print >should-be-empty && > test_line_count = 0 should-be-empty > ' > ++ find .git/objects -type f -print > ++ test_line_count = 0 should-be-empty > ++ test 3 '!=' 3 > +++ wc -l > ++ test 0 = 0 > + eval_ret=0 This is the key line. If it is 'eval_ret=1' (or other non-zero value), then the test failed, and the lines above it usually indicate where in the test snippet the failure occurred. -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: make test
> From: Johannes Sixt [mailto:j.s...@viscovery.net]
> Sent: Monday, October 15, 2012 1:18 PM
> To: Joachim Schmitz
> Cc: git@vger.kernel.org
> Subject: Re: make test
>
> Am 10/15/2012 13:00, schrieb Joachim Schmitz:
> >> From: Johannes Sixt [mailto:j.s...@viscovery.net]
> >> and if that does not give sufficient clues,
> >>
> >> $SHELL_PATH -x ./t-basic.sh -v -i
> >
> > not ok - 12 tests clean up even on failures
> > #...
> > + die
> >
> > Looks identical, except for the "die" at the end. And still leaves me
> > without a clue...
>
> When I do that it begins like this (I'm on Windows):
>
> D:\Src\mingw-git\t>bash -x t-basic.sh -v -i
> + test_description='Test the very basics part #1.
> ...
> '
> + . ./test-lib.sh
> ++ ORIGINAL_TERM=cygwin
> ++ test -z ''
> +++ pwd
> ++ TEST_DIRECTORY=/d/Src/mingw-git/t
> ++ test -z ''
> ++ TEST_OUTPUT_DIRECTORY=/d/Src/mingw-git/t
> ++ GIT_BUILD_DIR=/d/Src/mingw-git/t/..
> ++ /d/Src/mingw-git/t/../git
> ++ test 1 '!=' 1
> ++ . /d/Src/mingw-git/t/../GIT-BUILD-OPTIONS
> +++ SHELL_PATH=/bin/sh
> +++ PERL_PATH=/usr/bin/perl
> +++ DIFF=diff
> +++ PYTHON_PATH=/usr/bin/python
> +++ TAR=tar
> ...
>
> It seems you need a shell that is verbose under -x.
Erm, no, I left that part out...
+ . ./test-lib.sh
++ ORIGINAL_TERM=dumb
++ test -z ''
+++ pwd
++ TEST_DIRECTORY=/home/jojo/git/git/t
++ test -z ''
++ TEST_OUTPUT_DIRECTORY=/home/jojo/git/git/t
++ GIT_BUILD_DIR=/home/jojo/git/git/t/..
++ /home/jojo/git/git/t/../git
++ test 1 '!=' 1
++ . /home/jojo/git/git/t/../GIT-BUILD-OPTIONS
+++ SHELL_PATH=/bin/sh
+++ PERL_PATH=/usr/local/bin/perl
+++ DIFF=diff
+++ PYTHON_PATH=/usr/local/bin/python
+++ TAR=tar
+++ NO_CURL=
+++ USE_LIBPCRE=
+++ NO_PERL=
+++ NO_PYTHON=
+++ NO_UNIX_SOCKETS=
+++ GIT_TEST_CMP_USE_COPIED_CONTEXT=YesPlease
+++ NO_GETTEXT=
+++ GETTEXT_POISON=
++ export PERL_PATH SHELL_PATH
++ case "$GIT_TEST_TEE_STARTED, $* " in
++ LANG=C
++ LC_ALL=C
++ PAGER=cat
++ TZ=UTC
++ TERM=dumb
++ export LANG LC_ALL PAGER TERM TZ
++ EDITOR=:
+++ /usr/local/bin/perl -e '
my @env = keys %ENV;
my $ok = join("|", qw(
TRACE
DEBUG
USE_LOOKUP
TEST
.*_TEST
PROVE
VALGRIND
PERF_AGGREGATING_LATER
));
my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
print join("\n", @vars);
'
++ unset VISUAL EMAIL LANGUAGE COLUMNS GIT_AUTHOR_NAME GIT_MERGE_AUTOEDIT
GIT_EXEC_PATH GIT_ATTR_NOSYSTEM GIT_MERGE_VERBOSITY
GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_AUTHOR_EMAIL GIT_COMMITTER_NAME
GIT_COMMITTER_EMAIL
++ unset XDG_CONFIG_HOME
++ GIT_AUTHOR_EMAIL=aut...@example.com
++ GIT_AUTHOR_NAME='A U Thor'
++ GIT_COMMITTER_EMAIL=commit...@example.com
++ GIT_COMMITTER_NAME='C O Mitter'
++ GIT_MERGE_VERBOSITY=5
++ GIT_MERGE_AUTOEDIT=no
++ export GIT_MERGE_VERBOSITY GIT_MERGE_AUTOEDIT
++ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
++ export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
++ export EDITOR
++ expr ' ' : '.* --valgrind '
++ test -n ''
++ unset CDPATH
++ unset GREP_OPTIONS
++ case $(echo $GIT_TRACE |tr "[A-Z]" "[a-z]") in
+++ echo
+++ tr '[A-Z]' '[a-z]'
++ _x05='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
++
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-
f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0
-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
++ _z40=
++ LF='
'
++ export _x05 _x40 _z40 LF
++ '[' xdumb '!=' xdumb ']'
++ test 2 -ne 0
++ case "$1" in
++ verbose=t
++ shift
++ test 1 -ne 0
++ case "$1" in
++ immediate=t
++ shift
++ test 0 -ne 0
++ test -n ''
++ test 'Test the very basics part #1.
The rest of the test suite does not check the basic operation of git
plumbing commands to work very carefully. Their job is to concentrate
on tricky features that caused bugs in the past to detect regression.
This test runs very basic features, like registering things in cache,
writing tree, etc.
Note that this test *deliberately* hard-codes many expected object
IDs. When object ID computation changes, like in the previous case of
swapping compression and hashing order, the person who is making the
modification *should* take notice and update the test vectors here.
' '!=' ''
++ test '' = t
++ exec
++ exec
+
Re: make test
Am 10/15/2012 13:00, schrieb Joachim Schmitz: >> From: Johannes Sixt [mailto:j.s...@viscovery.net] >> and if that does not give sufficient clues, >> >> $SHELL_PATH -x ./t-basic.sh -v -i > > not ok - 12 tests clean up even on failures > #... > + die > > Looks identical, except for the "die" at the end. And still leaves me without > a clue... When I do that it begins like this (I'm on Windows): D:\Src\mingw-git\t>bash -x t-basic.sh -v -i + test_description='Test the very basics part #1. ... ' + . ./test-lib.sh ++ ORIGINAL_TERM=cygwin ++ test -z '' +++ pwd ++ TEST_DIRECTORY=/d/Src/mingw-git/t ++ test -z '' ++ TEST_OUTPUT_DIRECTORY=/d/Src/mingw-git/t ++ GIT_BUILD_DIR=/d/Src/mingw-git/t/.. ++ /d/Src/mingw-git/t/../git ++ test 1 '!=' 1 ++ . /d/Src/mingw-git/t/../GIT-BUILD-OPTIONS +++ SHELL_PATH=/bin/sh +++ PERL_PATH=/usr/bin/perl +++ DIFF=diff +++ PYTHON_PATH=/usr/bin/python +++ TAR=tar ... It seems you need a shell that is verbose under -x. -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
