{"thread":{"id":"9988","subject":"[StGit PATCH] New policy: Only use test_expect_failure for broken tests","startedAt":"2007-09-23T23:55:33Z","lastAt":"2007-09-23T23:55:33Z","messageCount":1,"participants":["Karl Hasselström"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"53875","messageId":"20070923235239.26457.939.stgit@yoghurt","threadId":"9988","inReplyTo":null,"subject":"[StGit PATCH] New policy: Only use test_expect_failure for broken tests","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-09-23T23:55:33Z","receivedAt":"2007-09-23T23:55:33Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"New policy for tests:\n\n  * For subtests whose commands fail when the test succeeds, use\n    test_expect_success and shell negation (!). test_expect_failure is\n    to be used only for subtests that are known to be broken.\n\n  * Patches that introduce a subtest failure must change that subtest\n    to use test_expect_failure.\n\nThe reason for this change is to ensure that the test suite passes at\nall times, even in the middle of disruptive rewrites spread over\nseveral commits.\n\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\nWhile rebasing David's conflict series on top of his new top/bottom\nremoval series, I found it very irritating and time-consuming to deal\nwith patches that intentionally broke the test suite. That won't\nhappen again with this policy in place.\n\n t/README                      |    9 ++++-----\n t/t0001-subdir-branches.sh    |    9 +++++----\n t/t1000-branch-create.sh      |    8 ++++----\n t/t1001-branch-rename.sh      |    4 ++--\n t/t1002-branch-clone.sh       |    4 ++--\n t/t1200-push-modified.sh      |    4 ++--\n t/t1202-push-undo.sh          |    4 ++--\n t/t2000-sync.sh               |    8 ++++----\n t/t2100-pull-policy-fetch.sh  |    4 ++--\n t/t2102-pull-policy-rebase.sh |   12 ++++++------\n t/t2200-rebase.sh             |    4 ++--\n 11 files changed, 35 insertions(+), 35 deletions(-)\n\n\ndiff --git a/t/README b/t/README\nindex d88bad2..77f0b6c 100644\n--- a/t/README\n+++ b/t/README\n@@ -162,11 +162,10 @@ library for your script to use.\n    This is the opposite of test_expect_success.  If <script>\n    yields success, test is considered a failure.\n \n-   Example:\n-\n-\ttest_expect_failure \\\n-\t    'git-update-index without --add should fail adding.' \\\n-\t    'git-update-index should-be-empty'\n+   This should _not_ be used for tests that succeed when their\n+   commands fail -- use test_expect_success and shell negation (!) for\n+   that. test_expect_failure is for cases when a test is known to be\n+   broken.\n \n  - test_debug <script>\n \ndiff --git a/t/t0001-subdir-branches.sh b/t/t0001-subdir-branches.sh\nindex 1685233..0eed3a4 100755\n--- a/t/t0001-subdir-branches.sh\n+++ b/t/t0001-subdir-branches.sh\n@@ -38,10 +38,11 @@ test_expect_success 'Try new form of id with slashy branch' \\\n    stg id foo@x/y/z &&\n    stg id foo@x/y/z//top'\n \n-test_expect_failure 'Try old id with slashy branch' \\\n-  'stg id foo/ ||\n-   stg id foo/top ||\n-   stg id foo@x/y/z/top'\n+test_expect_success 'Try old id with slashy branch' '\n+   ! stg id foo/ &&\n+   ! stg id foo/top &&\n+   ! stg id foo@x/y/z/top\n+   '\n \n test_expect_success 'Create patch in slashy branch' \\\n   'echo \"bar\" >> foo.txt &&\ndiff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh\nindex e920e93..848686c 100755\n--- a/t/t1000-branch-create.sh\n+++ b/t/t1000-branch-create.sh\n@@ -18,9 +18,9 @@ test_expect_success \\\n     mkdir -p .git/patches && touch .git/patches/foo\n '\n \n-test_expect_failure \\\n+test_expect_success \\\n     'Try to create an stgit branch with a spurious patches/ entry' '\n-    stg branch -c foo\n+    ! stg branch -c foo\n '\n \n test_expect_success \\\n@@ -35,9 +35,9 @@ test_expect_success \\\n     cp .git/refs/heads/master .git/refs/heads/foo\n '\n \n-test_expect_failure \\\n+test_expect_success \\\n     'Try to create an stgit branch with an existing git branch by that name' '\n-    stg branch -c foo\n+    ! stg branch -c foo\n '\n \n test_expect_success \\\ndiff --git a/t/t1001-branch-rename.sh b/t/t1001-branch-rename.sh\nindex 285440f..dd12132 100755\n--- a/t/t1001-branch-rename.sh\n+++ b/t/t1001-branch-rename.sh\n@@ -17,9 +17,9 @@ test_expect_success \\\n      stg new p1 -m \"p1\"\n '\n \n-test_expect_failure \\\n+test_expect_success \\\n     'Rename the current stgit branch' \\\n-    'stg branch -r foo bar\n+    '! stg branch -r foo bar\n '\n \n test_expect_success \\\ndiff --git a/t/t1002-branch-clone.sh b/t/t1002-branch-clone.sh\nindex 1d7fc39..b0087e9 100755\n--- a/t/t1002-branch-clone.sh\n+++ b/t/t1002-branch-clone.sh\n@@ -18,10 +18,10 @@ test_expect_success \\\n     git commit -a -m bar\n     '\n \n-test_expect_failure \\\n+test_expect_success \\\n     'Try to create a patch in a GIT branch' \\\n     '\n-    stg new p0 -m \"p0\"\n+    ! stg new p0 -m \"p0\"\n     '\n \n test_expect_success \\\ndiff --git a/t/t1200-push-modified.sh b/t/t1200-push-modified.sh\nindex 0e408d0..cfec696 100755\n--- a/t/t1200-push-modified.sh\n+++ b/t/t1200-push-modified.sh\n@@ -47,9 +47,9 @@ test_expect_success \\\n      )\n \"\n \n-test_expect_failure \\\n+test_expect_success \\\n     'Attempt to push the first of those patches without --merged' \\\n-    \"(cd bar && stg push\n+    \"(cd bar && ! stg push\n      )\n \"\n \ndiff --git a/t/t1202-push-undo.sh b/t/t1202-push-undo.sh\nindex 335b554..039103a 100755\n--- a/t/t1202-push-undo.sh\n+++ b/t/t1202-push-undo.sh\n@@ -40,10 +40,10 @@ test_expect_success \\\n \tstg pop --all\n \t'\n \n-test_expect_failure \\\n+test_expect_success \\\n \t'Push the second patch with conflict' \\\n \t'\n-\tstg push bar\n+\t! stg push bar\n \t'\n \n test_expect_success \\\ndiff --git a/t/t2000-sync.sh b/t/t2000-sync.sh\nindex f831df7..484dbab 100755\n--- a/t/t2000-sync.sh\n+++ b/t/t2000-sync.sh\n@@ -106,10 +106,10 @@ test_expect_success \\\n     test $(cat bar2.txt) = \"bar2\"\n     '\n \n-test_expect_failure \\\n+test_expect_success \\\n     'Synchronise the first two patches with the master branch (to fail)' \\\n     '\n-    stg sync -B master -a\n+    ! stg sync -B master -a\n     '\n \n test_expect_success \\\n@@ -124,10 +124,10 @@ test_expect_success \\\n     [ \"$(echo $(stg unapplied))\" = \"\" ]\n     '\n \n-test_expect_failure \\\n+test_expect_success \\\n     'Synchronise the third patch with the exported series (to fail)' \\\n     '\n-    stg sync -s patches-master/series p3\n+    ! stg sync -s patches-master/series p3\n     '\n \n test_expect_success \\\ndiff --git a/t/t2100-pull-policy-fetch.sh b/t/t2100-pull-policy-fetch.sh\nindex e1398a3..1f50069 100755\n--- a/t/t2100-pull-policy-fetch.sh\n+++ b/t/t2100-pull-policy-fetch.sh\n@@ -65,8 +65,8 @@ test_expect_success \\\n     (cd clone && stg commit && stg new c2 -m c2 &&\n      echo a >> file && stg refresh)\n     '\n-test_expect_failure \\\n+test_expect_success \\\n     'Try to  and commit a patch in clone' \\\n-    '(cd clone && stg pull)'\n+    '(cd clone && ! stg pull)'\n \n test_done\ndiff --git a/t/t2102-pull-policy-rebase.sh b/t/t2102-pull-policy-rebase.sh\nindex 670673d..b2fbfcf 100755\n--- a/t/t2102-pull-policy-rebase.sh\n+++ b/t/t2102-pull-policy-rebase.sh\n@@ -46,18 +46,18 @@ test_expect_success \\\n     stg branch stack && stg commit && stg new c2 -m c2 &&\n      echo a >> file && stg refresh\n     '\n-test_expect_failure \\\n+test_expect_success \\\n     'Try to pull/rebase now that stack base has moved' \\\n-    'stg pull'\n+    '! stg pull'\n \n test_expect_success \\\n     'Force the pull/rebase, but do not push yet' \\\n     'stg pull --force --nopush'\n-test_expect_failure \\\n+test_expect_success \\\n     '...check we lost the committed patch' \\\n-    'test -e file'\n-test_expect_failure \\\n+    '! test -e file'\n+test_expect_success \\\n     '...and check we get a conflict while pushing' \\\n-    'stg push'\n+    '! stg push'\n \n test_done\ndiff --git a/t/t2200-rebase.sh b/t/t2200-rebase.sh\nindex c142e08..ec2a104 100755\n--- a/t/t2200-rebase.sh\n+++ b/t/t2200-rebase.sh\n@@ -31,10 +31,10 @@ test_expect_success \\\n \ttest `stg applied | wc -l` = 1\n \t'\n \n-test_expect_failure \\\n+test_expect_success \\\n \t'Attempt rebase to non-existing commit' \\\n \t'\n-\tstg rebase not-a-ref\n+\t! stg rebase not-a-ref\n \t'\n \n test_expect_success \\\n"}]}