Fix PCT failures #199

chriskilding · 2020-06-30T13:14:04Z

Fix the PCT failures unearthed in jenkinsci/bom#179

To do:

Update test code
Test the fixes with PCT locally

src/test/java/jenkins/branch/CustomOrganizationFolderDescriptorTest.java

jglick

Right code to patch at least.

src/test/java/jenkins/branch/CustomOrganizationFolderDescriptorTest.java

src/test/java/jenkins/branch/OrganizationFolderTest.java

jglick · 2020-07-01T16:23:12Z

Thanks, looking close.

chriskilding · 2020-07-01T18:17:50Z

I think I've got the lot now, although the build is still sensitive to timeouts.

How would I test this locally with PCT?

jglick · 2020-07-01T19:46:43Z

How would I test this locally with PCT?

https://github.com/jenkinsci/plugin-compat-tester#configuration there is an option to specify a local directory to use for the plugin sources, rather than checking out a tag.

If you happen to know the specific extensions causing the failures, it may be easier to just add that other plugin to the test classpath.

chriskilding · 2020-07-02T10:08:18Z

I've run PCT against this with the following arguments:

docker run --rm -v maven-repo:/root/.m2 -v $(pwd)/out:/pct/out -e CHECKOUT_SRC=https://github.com/chriskilding/branch-api-plugin.git -e VERSION=fix-pct-failures jenkins/pct

The tests this was meant to fix seem to be fixed.

A couple of unrelated tests in branch-api failed, but I think it could just be some encoding issue running in the Docker PCT harness. Log below:

[ERROR] jenkins.branch.WorkspaceLocatorImplTest.deleteOffline  Time elapsed: 5.664 s  <<< ERROR!
hudson.model.Failure: ‘$’ is an unsafe character
	at jenkins.model.Jenkins.checkGoodName(Jenkins.java:4031)
	at hudson.model.ItemGroupMixIn.createProject(ItemGroupMixIn.java:316)
	at jenkins.model.Jenkins.createProject(Jenkins.java:3002)
	at jenkins.model.Jenkins.createProject(Jenkins.java:2998)
	at jenkins.model.Jenkins.createProject(Jenkins.java:3033)
	at org.jvnet.hudson.test.JenkinsRule.createProject(JenkinsRule.java:839)
	at org.jvnet.hudson.test.JenkinsRule.createFreeStyleProject(JenkinsRule.java:856)
	at jenkins.branch.WorkspaceLocatorImplTest.deleteOffline(WorkspaceLocatorImplTest.java:218)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54)
	at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:594)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:288)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:282)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.lang.Thread.run(Thread.java:748)

[ERROR] jenkins.branch.WorkspaceLocatorImplTest.locate  Time elapsed: 2.927 s  <<< ERROR!
hudson.model.Failure: ‘%’ is an unsafe character
	at jenkins.model.Jenkins.checkGoodName(Jenkins.java:4031)
	at hudson.model.ItemGroupMixIn.createProject(ItemGroupMixIn.java:316)
	at jenkins.model.Jenkins.createProject(Jenkins.java:3002)
	at jenkins.model.Jenkins.createProject(Jenkins.java:2998)
	at jenkins.model.Jenkins.createProject(Jenkins.java:3033)
	at org.jvnet.hudson.test.JenkinsRule.createProject(JenkinsRule.java:839)
	at org.jvnet.hudson.test.JenkinsRule.createFreeStyleProject(JenkinsRule.java:856)
	at jenkins.branch.WorkspaceLocatorImplTest.locate(WorkspaceLocatorImplTest.java:159)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54)
	at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:594)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:288)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:282)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.lang.Thread.run(Thread.java:748)

[ERROR] jenkins.branch.WorkspaceLocatorImplTest.uniquification  Time elapsed: 0.406 s  <<< ERROR!
hudson.model.Failure: ‘$’ is an unsafe character
	at jenkins.model.Jenkins.checkGoodName(Jenkins.java:4031)
	at hudson.model.ItemGroupMixIn.createProject(ItemGroupMixIn.java:316)
	at jenkins.model.Jenkins.createProject(Jenkins.java:3002)
	at jenkins.model.Jenkins.createProject(Jenkins.java:2998)
	at jenkins.model.Jenkins.createProject(Jenkins.java:3033)
	at org.jvnet.hudson.test.JenkinsRule.createProject(JenkinsRule.java:839)
	at org.jvnet.hudson.test.JenkinsRule.createFreeStyleProject(JenkinsRule.java:856)
	at jenkins.branch.WorkspaceLocatorImplTest.uniquification(WorkspaceLocatorImplTest.java:236)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54)
	at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:594)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:288)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:282)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.lang.Thread.run(Thread.java:748)

jglick · 2020-07-02T12:29:25Z

Not sure about the WorkspaceLocatorImplTest failures.

EventsTest timeout here smells like a flake.

jglick

Still weakening existing tests a bit, but close.

src/test/java/jenkins/branch/CustomOrganizationFolderDescriptorTest.java

src/test/java/jenkins/branch/matchers/Extracting.java

jglick

OK for me, assuming you retest in PCT.

chriskilding · 2020-07-03T09:34:32Z

Retested. I've since found 2 more tests that had also failed in the pipeline-model-definition fork of BOM (https://ci.jenkins.io/blue/organizations/jenkins/Tools%2Fbom/detail/PR-179/27/pipeline/#step-6960-log-1332), which I had overlooked. They have no error message:

integration.EventsTest.given_multibranch_when_oneEventBlocking_then_otherEventsProcessed
jenkins.branch.CustomOrganizationFolderDescriptorTest.dynamicLoad2

There's also the 3 tests with "Failure ‘$’ is an unsafe character" errors not mentioned in the PMD run, and probably just caused by the harness:

jenkins.branch.WorkspaceLocatorImplTest.deleteOffline
jenkins.branch.WorkspaceLocatorImplTest.locate
jenkins.branch.WorkspaceLocatorImplTest.uniquification

chriskilding · 2020-07-03T11:13:24Z

If I run dynamicLoad2 in my IDE, without PCT, I see the assumption assumeThat(CustomOrganizationFolderDescriptor.SHOW_GENERIC, is(true)); fails and the test is ignored.

Is Surefire treating an assumption failure (which should produce an ignored test) as a full test failure in the PCT harness?

jglick · 2020-07-06T17:06:59Z

Is Surefire treating an assumption failure (which should produce an ignored test) as a full test failure in the PCT harness?

Should not. Anyway I checked https://ci.jenkins.io/job/Tools/job/bom/job/PR-179/27/testReport/ and do not see any failures listed from what you mention in #199 (comment): they are all either passed or skipped.

jglick · 2020-07-06T17:07:45Z

tests that had also failed in PMD's run

Is there a reference? What is “PMD” in this context?

chriskilding · 2020-07-07T09:49:07Z

PMD was the pipeline-model-definition fork of BOM. Don't know why I abbreviated it 🤦

I was thinking of the same test report as you, but as you've pointed out, while the two tests appeared in the build log as failures (https://ci.jenkins.io/blue/organizations/jenkins/Tools%2Fbom/detail/PR-179/27/pipeline/#step-6960-log-1332) they didn't appear as failures in the test report. If we're going by the test report tab then we can say they're not a problem.

jglick · 2020-07-07T12:51:03Z

Well Surefire first prints

[WARNING] Tests run: 80, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 120.09 s - in integration.EventsTest

then later

lists it under

Failed: 11
- integration.EventsTest.given_multibranch_when_oneEventBlocking_then_otherEventsProcessed
…

but I guess that is just a buglet in Surefire display.

bitwiseman · 2020-07-07T22:55:41Z

There's also the 3 tests with "Failure ‘$’ is an unsafe character" errors not mentioned in the PMD run, and probably just caused by the harness:

...

Each of these looks like test failures due to this change: jenkinsci/jenkins@f176292

jglick · 2020-07-08T15:59:23Z

jenkinsci/jenkins#4684 for the better link/backlink. Those issues look distinct. For whatever reason they are not currently failing in bom PCT, though it would be good to deal with them as well (perhaps in another PR). Probably only test failures, not production regressions, since the test is just trying to simulate MultiBranchProject children with funny names which in reality would go through a different code path.

jglick · 2020-07-08T16:05:35Z

For whatever reason

…because this patch is not in 2.235, only 2.237 and proposed for 2.235.2. I will look at fixing but this would be in a separate PR, not needed immediately.

dwnusbaum

Looks good to me, will trigger a rebuild now that #200 has been merged.

dwnusbaum · 2020-07-07T21:05:06Z

src/test/java/jenkins/branch/CustomOrganizationFolderDescriptorTest.java

@@ -212,4 +212,4 @@ public String getDisplayName() {
        return names;
    }

-}
+}


Please try to avoid unnecessary formatting changes.

Suggested change

}

}

dwnusbaum · 2020-07-08T20:57:25Z

src/test/java/jenkins/branch/matchers/Extracting.java

+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
+
+public class Extracting<Element, Property> extends TypeSafeDiagnosingMatcher<Iterable<Element>> {


Personally I would prefer to not add a utility like this to this specific plugin. I have wanted something like this occasionally though; perhaps it could be added to jenkins-test-harness.

#199 (comment)

Fix PCT failures

1f9f869

chriskilding commented Jun 30, 2020

View reviewed changes

src/test/java/jenkins/branch/CustomOrganizationFolderDescriptorTest.java Outdated Show resolved Hide resolved

Patch up 2 more tests

e83ec5b

chriskilding mentioned this pull request Jun 30, 2020

Add pipeline-model-definition plugin jenkinsci/bom#179

Merged

jglick requested changes Jun 30, 2020

View reviewed changes

Use hasItems not contains, match concrete descriptors where possible

cddec39

chriskilding commented Jul 1, 2020

View reviewed changes

src/test/java/jenkins/branch/OrganizationFolderTest.java Outdated Show resolved Hide resolved

chriskilding added 2 commits July 1, 2020 17:30

Remove last contains(instanceOf(...))

87fecef

Fix for the 'ensure 0 factories present' assertions

79b1a62

chriskilding marked this pull request as ready for review July 2, 2020 11:00

jglick requested changes Jul 2, 2020

View reviewed changes

chriskilding added 2 commits July 2, 2020 16:16

Updates

9e71d42

Use assumeThat in CustomOrganizationFolderDescriptorTest

fbd5217

jglick approved these changes Jul 2, 2020

View reviewed changes

dwnusbaum requested review from dwnusbaum and bitwiseman July 7, 2020 13:14

jglick mentioned this pull request Jul 8, 2020

[JENKINS-61956] Adapt test to stricter enforcement of createFreeStyleProject arg #200

Merged

dwnusbaum approved these changes Jul 8, 2020

View reviewed changes

dwnusbaum closed this Jul 8, 2020

dwnusbaum reopened this Jul 8, 2020

dwnusbaum merged commit beda204 into jenkinsci:master Jul 8, 2020

jglick mentioned this pull request Oct 16, 2020

Fix OrganizationFolderTest.verifyBranchPropertiesAppliedOnNewProjects in PCT #222

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix PCT failures #199

Fix PCT failures #199

chriskilding commented Jun 30, 2020 •

edited

Loading

jglick left a comment

jglick commented Jul 1, 2020

chriskilding commented Jul 1, 2020

jglick commented Jul 1, 2020

chriskilding commented Jul 2, 2020

jglick commented Jul 2, 2020

jglick left a comment

jglick left a comment

chriskilding commented Jul 3, 2020 •

edited

Loading

chriskilding commented Jul 3, 2020

jglick commented Jul 6, 2020

jglick commented Jul 6, 2020

chriskilding commented Jul 7, 2020

jglick commented Jul 7, 2020

bitwiseman commented Jul 7, 2020 •

edited

Loading

jglick commented Jul 8, 2020

jglick commented Jul 8, 2020

dwnusbaum left a comment

dwnusbaum Jul 7, 2020

dwnusbaum Jul 8, 2020

jglick Jul 9, 2020

Fix PCT failures #199

Fix PCT failures #199

Conversation

chriskilding commented Jun 30, 2020 • edited Loading

jglick left a comment

Choose a reason for hiding this comment

jglick commented Jul 1, 2020

chriskilding commented Jul 1, 2020

jglick commented Jul 1, 2020

chriskilding commented Jul 2, 2020

jglick commented Jul 2, 2020

jglick left a comment

Choose a reason for hiding this comment

jglick left a comment

Choose a reason for hiding this comment

chriskilding commented Jul 3, 2020 • edited Loading

chriskilding commented Jul 3, 2020

jglick commented Jul 6, 2020

jglick commented Jul 6, 2020

chriskilding commented Jul 7, 2020

jglick commented Jul 7, 2020

bitwiseman commented Jul 7, 2020 • edited Loading

jglick commented Jul 8, 2020

jglick commented Jul 8, 2020

dwnusbaum left a comment

Choose a reason for hiding this comment

dwnusbaum Jul 7, 2020

Choose a reason for hiding this comment

dwnusbaum Jul 8, 2020

Choose a reason for hiding this comment

jglick Jul 9, 2020

Choose a reason for hiding this comment

chriskilding commented Jun 30, 2020 •

edited

Loading

chriskilding commented Jul 3, 2020 •

edited

Loading

bitwiseman commented Jul 7, 2020 •

edited

Loading