{"thread":{"id":"19162","subject":"[JGIT PATCH 7/6] BROKEN: Add a zero line context test for diff.DiffFormatter","startedAt":"2009-05-03T00:05:40Z","lastAt":"2009-05-05T22:19:46Z","messageCount":8,"participants":["Shawn O. Pearce","Miles Bader","Robin Rosenberg","Ferry Huberts (Pelagic)","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"112894","messageId":"20090503000540.GN23604@spearce.org","threadId":"19162","inReplyTo":null,"subject":"[JGIT PATCH 7/6] BROKEN: Add a zero line context test for diff.DiffFormatter","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-05-03T00:05:40Z","receivedAt":"2009-05-03T00:05:40Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Signed-off-by: Shawn O. Pearce \n---\n\n This test currently fails because it shows the difference between\n the way CGit and JGit number a zero context line patch.\n\n Either we hack JGit to match CGit here, or we modify the test\n vector, or CGit agrees there's a bug and fixes their code, and we\n modify the test vector.\n\n .../org/spearce/jgit/diff/testContext0.out | 18 ++++++++++++++++++\n .../spearce/jgit/diff/DiffFormatterReflowTest.java | 6 ++++++\n 2 files changed, 24 insertions(+), 0 deletions(-)\n create mode 100644 org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/diff/testContext0.out\n\ndiff --git a/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/diff/testContext0.out b/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/diff/testContext0.out\nnew file mode 100644\nindex 0000000..d36e3fa\n--- /dev/null\n+++ b/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/diff/testContext0.out\n@@ -0,0 +1,18 @@\n+diff --git a/X b/X\n+index a3648a1..2d44096 100644\n+--- a/X\n++++ b/X\n+@@ -2,0 +3 @@\n++c\n+@@ -17,2 +17,0 @@\n+-r\n+-s\n+@@ -23,2 +22,6 @@\n+-x\n+-y\n++0\n++1\n++2\n++3\n++4\n++5\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/diff/DiffFormatterReflowTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/diff/DiffFormatterReflowTest.java\nindex f47282c..5d2ee40 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/diff/DiffFormatterReflowTest.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/diff/DiffFormatterReflowTest.java\n@@ -74,6 +74,12 @@ public void testNegativeContextFails() throws IOException {\n \t\t}\n \t}\n \n+\tpublic void testContext0() throws IOException {\n+\t\tinit(\"X\");\n+\t\tfmt.setContext(0);\n+\t\tassertFormatted();\n+\t}\n+\n \tpublic void testContext1() throws IOException {\n \t\tinit(\"X\");\n \t\tfmt.setContext(1);\n-- \n1.6.3.rc4.190.g4648\n"},{"id":"112895","messageId":"20090503001423.GO23604@spearce.org","threadId":"19162","inReplyTo":"20090503000540.GN23604@spearce.org","subject":"[JGIT PATCH 8/6] Fix zero context insert and delete hunk headers to match CGit","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-05-03T00:14:23Z","receivedAt":"2009-05-03T00:14:23Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Signed-off-by: Shawn O. Pearce \n---\n \"Shawn O. Pearce\" wrote:\n > This test currently fails because it shows the difference between\n > the way CGit and JGit number a zero context line patch.\n > \n > Either we hack JGit to match CGit here\n\n And here is that hack. It just feels wrong to me that I need\n to subtract 1 from the Edit region's line numbers, *only* when\n context is 0, in order to get the same output as CGit.\n\n .../src/org/spearce/jgit/diff/DiffFormatter.java | 23 ++++++++++++++++---\n 1 files changed, 19 insertions(+), 4 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/diff/DiffFormatter.java b/org.spearce.jgit/src/org/spearce/jgit/diff/DiffFormatter.java\nindex 97db9a2..9930904 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/diff/DiffFormatter.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/diff/DiffFormatter.java\n@@ -120,7 +120,7 @@ private void formatEdits(final OutputStream out, final RawText a,\n \t\t\tfinal int aEnd = Math.min(a.size(), endEdit.getEndA() + context);\n \t\t\tfinal int bEnd = Math.min(b.size(), endEdit.getEndB() + context);\n \n-\t\t\twriteHunkHeader(out, aCur, aEnd, bCur, bEnd, curIdx == 0);\n+\t\t\twriteHunkHeader(out, aCur, aEnd, bCur, bEnd, curEdit, curIdx == 0);\n \n \t\t\twhile (aCur < aEnd || bCur < bEnd) {\n \t\t\t\tif (aCur < curEdit.getBeginA() || endIdx + 1 < curIdx) {\n@@ -141,9 +141,24 @@ private void formatEdits(final OutputStream out, final RawText a,\n \t\t}\n \t}\n \n-\tprivate void writeHunkHeader(final OutputStream out, final int aCur,\n-\t\t\tfinal int aEnd, final int bCur, final int bEnd,\n-\t\t\tfinal boolean firstHunk) throws IOException {\n+\tprivate void writeHunkHeader(final OutputStream out, int aCur, int aEnd,\n+\t\t\tint bCur, int bEnd, final Edit curEdit, final boolean firstHunk)\n+\t\t\tthrows IOException {\n+\t\tif (context == 0) {\n+\t\t\tswitch (curEdit.getType()) {\n+\t\t\tcase INSERT:\n+\t\t\t\taCur--;\n+\t\t\t\taEnd--;\n+\t\t\t\tbreak;\n+\t\t\tcase DELETE:\n+\t\t\t\tbCur--;\n+\t\t\t\tbEnd--;\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\n \t\tout.write('@');\n \t\tout.write('@');\n \t\tif (firstHunk) {\n-- \n1.6.3.rc4.190.g4648\n"},{"id":"112899","messageId":"8763gjdn6r.fsf@catnip.gol.com","threadId":"19162","inReplyTo":"20090503001423.GO23604@spearce.org","subject":"Re: [JGIT PATCH 8/6] Fix zero context insert and delete hunk headers to match CGit","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2009-05-03T00:50:36Z","receivedAt":"2009-05-03T00:50:36Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"\"Shawn O. Pearce\" writes:\n> And here is that hack. It just feels wrong to me that I need\n> to subtract 1 from the Edit region's line numbers, *only* when\n> context is 0, in order to get the same output as CGit.\n\nA big comment saying \"this may look a bit funny, but it's the standard's\nfault: [text from std...]\" might help salve the wound...\n\n-Miles\n\n-- \n.Numeric stability is probably not all that important when you're guessing.\n"},{"id":"112911","messageId":"200905031025.53084.robin.rosenberg.lists@dewire.com","threadId":"19162","inReplyTo":"8763gjdn6r.fsf@catnip.gol.com","subject":"Re: [JGIT PATCH 8/6] Fix zero context insert and delete hunk headers to match CGit","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-05-03T08:25:52Z","receivedAt":"2009-05-03T08:25:52Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"söndag 03 maj 2009 02:50:36 skrev Miles Bader :\n> \"Shawn O. Pearce\" writes:\n> > And here is that hack. It just feels wrong to me that I need\n> > to subtract 1 from the Edit region's line numbers, *only* when\n> > context is 0, in order to get the same output as CGit.\n> \n> A big comment saying \"this may look a bit funny, but it's the standard's\n> fault: [text from std...]\" might help salve the wound...\n\n1) I agree. This need to be commented in the code.\n\n2) Do we need to fix it? I.e. is there a problem or just two different results,\nwhere one is correct, the other incorrect but harmless.\n\n3) We should have a convention like C Git for marking known breakages.\nOne option is FIXME, another it so go JUnit 4 and abuse the expected exception \nannotation (using it for declaring OK exceptions is pretty bad use anyway I think,\nso we might use it for something better), or perhaps the @Ignore annotation which\nis meant specifically for this and other cases. A FIXME can be implemented right\naway.\n\n-- robin\n"},{"id":"112912","messageId":"49FD5662.9070004@pelagic.nl","threadId":"19162","inReplyTo":"200905031025.53084.robin.rosenberg.lists@dewire.com","subject":"Re: [JGIT PATCH 8/6] Fix zero context insert and delete hunk headers to match CGit","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-05-03T08:31:30Z","receivedAt":"2009-05-03T08:31:30Z","isPatch":true,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"> 3) We should have a convention like C Git for marking known breakages.\n> One option is FIXME, another it so go JUnit 4 and abuse the expected exception \n> annotation (using it for declaring OK exceptions is pretty bad use anyway I think,\n> so we might use it for something better), or perhaps the @Ignore annotation which\n> is meant specifically for this and other cases. A FIXME can be implemented right\n> away.\n\nstandard pratice for junit would be to write a test case on what you would \nexpect to be _correct_ behaviour. obviously that test would then fail.\nit would be a know failure in the test suite. do not go ignoring it. it's \nbetter to keep being reminded that stuff doesn't work :-)\n"},{"id":"112914","messageId":"200905031124.08113.robin.rosenberg.lists@dewire.com","threadId":"19162","inReplyTo":"49FD5662.9070004@pelagic.nl","subject":"Re: [JGIT PATCH 8/6] Fix zero context insert and delete hunk headers to match CGit","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-05-03T09:24:07Z","receivedAt":"2009-05-03T09:24:07Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"söndag 03 maj 2009 10:31:30 skrev \"Ferry Huberts (Pelagic)\" :\n> > 3) We should have a convention like C Git for marking known breakages.\n> > One option is FIXME, another it so go JUnit 4 and abuse the expected exception \n> > annotation (using it for declaring OK exceptions is pretty bad use anyway I think,\n> > so we might use it for something better), or perhaps the @Ignore annotation which\n> > is meant specifically for this and other cases. A FIXME can be implemented right\n> > away.\n> \n> standard pratice for junit would be to write a test case on what you would \n> expect to be _correct_ behaviour. obviously that test would then fail.\n> it would be a know failure in the test suite. do not go ignoring it. it's \n> better to keep being reminded that stuff doesn't work :-)\n\nWhat I've see so far is that people start ignoring almost any failure, including new ones, when the test suites contains fails with \"known\" failues. The assumption is that the failed tests were the same as before.\n\nWorse, automated tests have a hard time telling the difference. Currently I ran\nthe jgit tests as part of the Eclipse plugin build and I want it to stop if there is a problem that we don't know of. \n\n\"Annotation\" of different kinds can be \"grepped\" for so we can find the broken\ncases separately and even refuse completion of release builds if we decide\non that. \n\nOur primary UI right now is the Eclipse JUnit tests runner and I don't want\nto be remined of Shawn's or whoever's bugs when trying to make sure I don't\nbreak anything. Red = *I* broke something or found something new. \n\nTestNG has a nice way of classifying tests so, we could mark failures as \"known failures\" and specifically exclude/include them when invoking the\nJUnit tests.\n\nBest is to fix before we apply the patches as happened this time. So this problem still remains theoretical :)\n\n-- robin\n\n\n-- robin\n"},{"id":"112915","messageId":"49FD63F0.5010809@pelagic.nl","threadId":"19162","inReplyTo":"200905031124.08113.robin.rosenberg.lists@dewire.com","subject":"Re: [JGIT PATCH 8/6] Fix zero context insert and delete hunk headers to match CGit","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-05-03T09:29:20Z","receivedAt":"2009-05-03T09:29:20Z","isPatch":true,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"fair enough :-)\n"},{"id":"113058","messageId":"20090505221946.GA20002@neumann","threadId":"19162","inReplyTo":"200905031124.08113.robin.rosenberg.lists@dewire.com","subject":"Re: [JGIT PATCH 8/6] Fix zero context insert and delete hunk headers to match CGit","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2009-05-05T22:19:46Z","receivedAt":"2009-05-05T22:19:46Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Sun, May 03, 2009 at 11:24:07AM +0200, Robin Rosenberg wrote:\n> söndag 03 maj 2009 10:31:30 skrev \"Ferry Huberts (Pelagic)\" :\n> > > 3) We should have a convention like C Git for marking known breakages.\n> > > One option is FIXME, another it so go JUnit 4 and abuse the expected exception \n> > > annotation (using it for declaring OK exceptions is pretty bad use anyway I think,\n> > > so we might use it for something better), or perhaps the @Ignore annotation which\n> > > is meant specifically for this and other cases. A FIXME can be implemented right\n> > > away.\n> > \n> > standard pratice for junit would be to write a test case on what you would \n> > expect to be _correct_ behaviour. obviously that test would then fail.\n> > it would be a know failure in the test suite. do not go ignoring it. it's \n> > better to keep being reminded that stuff doesn't work :-)\n> \n> What I've see so far is that people start ignoring almost any failure, including new ones, when the test suites contains fails with \"known\" failues. The assumption is that the failed tests were the same as before.\n> \n> Worse, automated tests have a hard time telling the difference. Currently I ran\n> the jgit tests as part of the Eclipse plugin build and I want it to stop if there is a problem that we don't know of. \n> \n> \"Annotation\" of different kinds can be \"grepped\" for so we can find the broken\n> cases separately and even refuse completion of release builds if we decide\n> on that. \n> \n> Our primary UI right now is the Eclipse JUnit tests runner and I don't want\n> to be remined of Shawn's or whoever's bugs when trying to make sure I don't\n> break anything. Red = *I* broke something or found something new. \n> \n> TestNG has a nice way of classifying tests so, we could mark failures as \"known failures\" and specifically exclude/include them when invoking the\n> JUnit tests.\n\nyou could use test suites to easily circumvent this in JUnit, even in\nJUnit 3.x.\n\nJust set up two test suites: one for the tests that should pass and\none for the tests with known breakages. That way you can run either\nonly the \"good\" tests or only the broken ones. Or even both, and you\ncan easily discern failures caused by known breakages from your new\nbreakages by looking at the tree of tests in eclipse's JUnit view.\n\n\nRegards,\nGábor\n"}]}