regression in subtest parsing between 4.2.1 an 4.3 #56

cburroughs · 2019-02-13T20:00:36Z

I'm trying to upgrade https://github.com/jenkinsci/tap-plugin to use the latest release of tap4j. When doing some I'm running into several failures with the plugins unit tests.

Specifically:

[ERROR] Failures: 
[ERROR] org.tap4j.plugin.flattentapfeature.TestFlattenTapResult.testStripFirstLevel(org.tap4j.plugin.flattentapfeature.TestFlattenTapResult)
[ERROR]   Run 1: TestFlattenTapResult.testStripFirstLevel:65->_test:192 expected:<5> but was:<3>
[ERROR]   Run 2: TestFlattenTapResult.testStripFirstLevel:65->_test:192 expected:<5> but was:<3>
[ERROR]   Run 3: TestFlattenTapResult.testStripFirstLevel:65->_test:192 expected:<5> but was:<3>
[ERROR]   Run 4: TestFlattenTapResult.testStripFirstLevel:65->_test:192 expected:<5> but was:<3>
[ERROR]   Run 5: TestFlattenTapResult.testStripFirstLevel:65->_test:192 expected:<5> but was:<3>
[INFO] 
[ERROR] org.tap4j.plugin.flattentapfeature.TestFlattenTapResult.testStripSecondLevel(org.tap4j.plugin.flattentapfeature.TestFlattenTapResult)
[ERROR]   Run 1: TestFlattenTapResult.testStripSecondLevel:89->_test:192 expected:<7> but was:<1>
[ERROR]   Run 2: TestFlattenTapResult.testStripSecondLevel:89->_test:192 expected:<7> but was:<1>
[ERROR]   Run 3: TestFlattenTapResult.testStripSecondLevel:89->_test:192 expected:<7> but was:<1>
[ERROR]   Run 4: TestFlattenTapResult.testStripSecondLevel:89->_test:192 expected:<7> but was:<1>
[ERROR]   Run 5: TestFlattenTapResult.testStripSecondLevel:89->_test:192 expected:<7> but was:<1>
[INFO] 
[ERROR] org.tap4j.plugin.flattentapfeature.TestFlattenTapResult.testStripSecondLevelIncompleteResult1(org.tap4j.plugin.flattentapfeature.TestFlattenTapResult)
[ERROR]   Run 1: TestFlattenTapResult.testStripSecondLevelIncompleteResult1:113->_test:192 expected:<7> but was:<1>
[ERROR]   Run 2: TestFlattenTapResult.testStripSecondLevelIncompleteResult1:113->_test:192 expected:<7> but was:<1>
[ERROR]   Run 3: TestFlattenTapResult.testStripSecondLevelIncompleteResult1:113->_test:192 expected:<7> but was:<1>
[ERROR]   Run 4: TestFlattenTapResult.testStripSecondLevelIncompleteResult1:113->_test:192 expected:<7> but was:<1>
[ERROR]   Run 5: TestFlattenTapResult.testStripSecondLevelIncompleteResult1:113->_test:192 expected:<7> but was:<1>
[INFO] 
[ERROR] org.tap4j.plugin.flattentapfeature.TestFlattenTapResult.testStripSecondLevelIncompleteResult2(org.tap4j.plugin.flattentapfeature.TestFlattenTapResult)
[ERROR]   Run 1: TestFlattenTapResult.testStripSecondLevelIncompleteResult2:134->_test:192 expected:<6> but was:<1>
[ERROR]   Run 2: TestFlattenTapResult.testStripSecondLevelIncompleteResult2:134->_test:192 expected:<6> but was:<1>
[ERROR]   Run 3: TestFlattenTapResult.testStripSecondLevelIncompleteResult2:134->_test:192 expected:<6> but was:<1>
[ERROR]   Run 4: TestFlattenTapResult.testStripSecondLevelIncompleteResult2:134->_test:192 expected:<6> but was:<1>
[ERROR]   Run 5: TestFlattenTapResult.testStripSecondLevelIncompleteResult2:134->_test:192 expected:<6> but was:<1>
[INFO] 
[ERROR] org.tap4j.plugin.stripsingleparent.TestStripSingleParent.testStripFirstLevel(org.tap4j.plugin.stripsingleparent.TestStripSingleParent)
[ERROR]   Run 1: TestStripSingleParent.testStripFirstLevel:56->_test:111 expected:<3> but was:<1>
[ERROR]   Run 2: TestStripSingleParent.testStripFirstLevel:56->_test:111 expected:<3> but was:<1>
[ERROR]   Run 3: TestStripSingleParent.testStripFirstLevel:56->_test:111 expected:<3> but was:<1>
[ERROR]   Run 4: TestStripSingleParent.testStripFirstLevel:56->_test:111 expected:<3> but was:<1>
[ERROR]   Run 5: TestStripSingleParent.testStripFirstLevel:56->_test:111 expected:<3> but was:<1>
[INFO] 
[ERROR] org.tap4j.plugin.stripsingleparent.TestStripSingleParent.testStripSecondLevel(org.tap4j.plugin.stripsingleparent.TestStripSingleParent)
[ERROR]   Run 1: TestStripSingleParent.testStripSecondLevel:72->_test:111 expected:<3> but was:<1>
[ERROR]   Run 2: TestStripSingleParent.testStripSecondLevel:72->_test:111 expected:<3> but was:<1>
[ERROR]   Run 3: TestStripSingleParent.testStripSecondLevel:72->_test:111 expected:<3> but was:<1>
[ERROR]   Run 4: TestStripSingleParent.testStripSecondLevel:72->_test:111 expected:<3> but was:<1>
[ERROR]   Run 5: TestStripSingleParent.testStripSecondLevel:72->_test:111 expected:<3> but was:<1>
[INFO] 
[INFO] 
[ERROR] Tests run: 48, Failures: 6, Errors: 0, Skipped: 0

I believe these are related to a change in how subtests are parsed between 4.2.1 and 4.3. I rigged up one of the plugin tests to print more output and looked at how TestFlattenTapResult#testStripFirstLevel behaved. This test takes a string representing TAP output like

        final String tap = "1..2\n" +
                "ok 1 1\n" +
                "  1..2\n" +
                "  ok 1 .1\n" +
                "  ok 2 .2\n" +
                "ok 2 2\n" +
                "  1..3\n" +
                "  ok 1 .1\n" +
                "  ok 2 .2\n" +
                "  ok 3 .3\n";

parses it using tap4j, and then attempts to "flatten" it. I looked at both what the test is evaluating, and the original test set before trying to flatten.

4.2.1

flattened

ok 1 1.1
ok 2 1.2
ok 3 2.1
ok 4 2.2
ok 5 2.3

original

1..2
ok 1 1
    1..2
    ok 1 .1
    ok 2 .2
ok 2 2
    1..3
    ok 1 .1
    ok 2 .2
    ok 3 .3

4.3

flattened

ok 1 1
ok 2 2.1
ok 3 2.2

original

1..2
ok 1 1
    1..2
    ok 1 .1
    ok 2 .2
ok 2 2

4.4.1

flattened

ok 1 1
ok 2 2.1
ok 3 2.2

original

1..2
ok 1 1
    1..2
    ok 1 .1
    ok 2 .2
ok 2 2

I'm not familiar with the details of TAP nor the "flattening" that the plugin does. But it appears that starting in 4.3 tap4j started ignoring everything after the start of the second subtest.

Rough cut of plugin test code:

diff --git a/pom.xml b/pom.xml
index f6f374b..9b3037b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -52,7 +52,7 @@
 		<jenkins.version>2.121.1</jenkins.version>
 		<java.level>8</java.level>
 		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
-		<tap4j.version>4.2.1</tap4j.version>
+		<tap4j.version>4.4.1</tap4j.version>
 		<junit.plugin.version>1.6</junit.plugin.version>
 	</properties>
 
diff --git a/src/test/java/org/tap4j/plugin/flattentapfeature/TestFlattenTapResult.java b/src/test/java/org/tap4j/plugin/flattentapfeature/TestFlattenTapResult.java
index 09741a0..5267dfb 100644
--- a/src/test/java/org/tap4j/plugin/flattentapfeature/TestFlattenTapResult.java
+++ b/src/test/java/org/tap4j/plugin/flattentapfeature/TestFlattenTapResult.java
@@ -24,6 +24,11 @@ import org.tap4j.plugin.TapPublisher;
 import org.tap4j.plugin.TapResult;
 import org.tap4j.plugin.TapTestResultAction;
 
+import org.tap4j.representer.Tap13Representer;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+
+
 /**
  * Tests for flatten TAP result configuration option.
  *
@@ -179,7 +184,7 @@ public class TestFlattenTapResult {
                 false, // verbose
                 true,  // showOnlyFailures
                 false, // stripSingleParents
-                true, // flattenTapResult
+                false, //true, // flattenTapResult  tmp: for looking at flat vs original
                 false); //skipIfBuildNotOk
 
         project.getPublishersList().add(publisher);
@@ -189,6 +194,11 @@ public class TestFlattenTapResult {
         TapTestResultAction action = build.getAction(TapTestResultAction.class);
         TapResult testResult = action.getTapResult();
 
+        Tap13Representer repr = new Tap13Representer();
+        System.out.println("~~~~~~~~~~~~");
+        System.out.print(repr.representData(testResult.getTestSets().get(0).getTestSet()));
+        System.out.println("~~~~~~~~~~~~");
+
         assertEquals(expectedTotal, testResult.getTotal());
 
         final TestSet testSet = testResult.getTestSets().get(0).getTestSet();
@@ -200,7 +210,7 @@ public class TestFlattenTapResult {
 
             int expectedTestNumber = testIndex +1;
 
-            if (printDescriptions) {
+            if (printDescriptions || true) {
                 System.out.printf("%d: %s\n", testNumber, description);
             }

The text was updated successfully, but these errors were encountered:

kinow · 2019-02-13T20:46:50Z

Happy at least the artefacts are working OK in maven central now. I have a slight memory of the changes related to this issues. In one issue, at least, it was pointed how we were parsing subtests completely incorrectly 😞

Will try to have another look this week and see what it should actually be (probably by using some Perl-fu).

Thanks a ton for the detailed bug report @cburroughs !

cburroughs · 2019-02-14T18:14:19Z

The primarily tool I'm working with now is node-tap, so I found its set of tests useful while puzzling this out: https://github.com/tapjs/tap-parser/tree/master/test/fixtures

kinow · 2019-03-01T01:03:57Z

Hmmm, had a quick look this afternoon, fixed javadocs errors while building, then tried writing a simple test, but found that I coudln't get it to produce the desired state... subtests might be utterly broken in the master branch - and possibly previous releases, haven't tested that yet.

japod · 2019-05-31T14:12:13Z

As stated in the above referenced issue #57 i believe the commit referenced there, a34d494, is indeed to blame. Updating existing tests always smells, and the linked update looks really strange: a34d494#diff-f5bf7967aaf5cfc38fc1e59eb3540a81 The original test was quite valid IMHO, and i do not see any explanation anywhere why the existing test for issue#17 must have been changed. Will see if i can re-introduce the original test case while keeping the implementation work for the rest of the test suite.

japod · 2019-05-31T14:58:34Z

After reading some more comments and docs, i tend to believe that it is probably the plugin code that needs update. The commit blamed above introduced a breaking change in tap4j that fixed sub-test processing, and this breaking change was not propagated to the plugin. I will have a look if i can do something to fix that over the weekend.

kinow · 2019-06-01T07:17:45Z

After reading some more comments and docs, i tend to believe that it is probably the plugin code that needs update.

Very likely. The plugin started as a - rather quick - proof of concept based on some other plug-in structure (TestNG maybe?), and the whole code base needs some re-work. I remember it did things like copying files around, and keeping maps in memory. That is probably affecting some users too.

If you have any improvements, feel free to submit PR's. I might have a go at updating it too soon (overseas at a conference until beginning of July)

japod · 2019-06-03T15:01:38Z

I have just opened jenkinsci/tap-plugin#24 that fixes the problematic tests and upgrades the tap4j dependency version to the latest greatest (4.4.2)

kinow self-assigned this Feb 13, 2019

kinow added the bug label Feb 13, 2019

beatngu13 mentioned this issue Mar 18, 2019

Subtests order/format change as of v4.3 #57

Open

kinow added this to the 4.4.3 milestone Jun 1, 2019

kinow added the help wanted label Jun 1, 2019

japod mentioned this issue Jun 3, 2019

tap4j dependency upgraded to 4.4.2 version jenkinsci/tap-plugin#24

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

regression in subtest parsing between 4.2.1 an 4.3 #56

regression in subtest parsing between 4.2.1 an 4.3 #56

cburroughs commented Feb 13, 2019

kinow commented Feb 13, 2019

cburroughs commented Feb 14, 2019

kinow commented Mar 1, 2019

japod commented May 31, 2019

japod commented May 31, 2019

kinow commented Jun 1, 2019

japod commented Jun 3, 2019

regression in subtest parsing between 4.2.1 an 4.3 #56

regression in subtest parsing between 4.2.1 an 4.3 #56

Comments

cburroughs commented Feb 13, 2019

4.2.1

flattened

original

4.3

flattened

original

4.4.1

flattened

original

kinow commented Feb 13, 2019

cburroughs commented Feb 14, 2019

kinow commented Mar 1, 2019

japod commented May 31, 2019

japod commented May 31, 2019

kinow commented Jun 1, 2019

japod commented Jun 3, 2019