Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JEP-210] Integration testing #252

Merged
merged 25 commits into from
Oct 4, 2018
Merged

Conversation

jglick
Copy link
Member

@jglick jglick commented Oct 3, 2018

@jglick
Copy link
Member Author

jglick commented Oct 3, 2018

Actually from a quick glance it looks like StepNodeTest failures are legitimate; looking.

A FlowNode was being sent to GraphListener.Synchronous without ArgumentsAction available.
I got a couple failures locally but am unclear how to interpret them.
Probably meaningless unless they are known to pass against released updates.
org.junit.ComparisonFailure: expected:<[sleep]> but was:<[// parallel]>
	at org.junit.Assert.assertEquals(Assert.java:115)
	at org.junit.Assert.assertEquals(Assert.java:144)
	at org.jenkinsci.plugins.workflow.cps.FlowDurabilityTest.assertIncludesNodes(FlowDurabilityTest.java:664)
	at org.jenkinsci.plugins.workflow.cps.FlowDurabilityTest$25.evaluate(FlowDurabilityTest.java:887)
java.lang.AssertionError: expected:<SUCCESS> but was:<FAILURE>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:118)
	at org.junit.Assert.assertEquals(Assert.java:144)
	at org.jenkinsci.plugins.workflow.cps.FlowDurabilityTest$30.evaluate(FlowDurabilityTest.java:1000)
@jglick
Copy link
Member Author

jglick commented Oct 3, 2018

Regarding PersistenceProblemsTest, I am indeed getting some failures locally which might reflect regressions, or might be flaky test issues etc.:

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertFalse(Assert.java:64)
	at org.junit.Assert.assertFalse(Assert.java:74)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.assertCompletedCleanly(PersistenceProblemsTest.java:77)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.runBasicBuild(PersistenceProblemsTest.java:115)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.runBasicBuild(PersistenceProblemsTest.java:121)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.lambda$completedNoNodesPersisted$5(PersistenceProblemsTest.java:189)
java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertFalse(Assert.java:64)
	at org.junit.Assert.assertFalse(Assert.java:74)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.assertCompletedCleanly(PersistenceProblemsTest.java:77)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.runBasicBuild(PersistenceProblemsTest.java:115)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.runBasicBuild(PersistenceProblemsTest.java:121)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.lambda$completedButWrongDoneStatus$7(PersistenceProblemsTest.java:210)
java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertFalse(Assert.java:64)
	at org.junit.Assert.assertFalse(Assert.java:74)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.assertCompletedCleanly(PersistenceProblemsTest.java:77)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.runBasicBuild(PersistenceProblemsTest.java:115)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.runBasicBuild(PersistenceProblemsTest.java:121)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.lambda$completedFinalFlowNodeNotPersisted$3(PersistenceProblemsTest.java:164)

I am not sure how seriously to take these since I get a different test failure when I simply use the same Jenkins core baseline and the most recent released deps of the affected plugins:

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertFalse(Assert.java:64)
	at org.junit.Assert.assertFalse(Assert.java:74)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.assertCompletedCleanly(PersistenceProblemsTest.java:59)
	at org.jenkinsci.plugins.workflow.cps.PersistenceProblemsTest.lambda$inProgressButProgramLoadFailure$19(PersistenceProblemsTest.java:321)

In other words, we have a situation where PCT is not getting regularly run here, and at least some failures are not related to JEP-210.

@jglick jglick changed the title JEP-210 integration testing [JEP-210] Integration testing and minor follow-up fixes Oct 3, 2018
@svanoort
Copy link
Member

svanoort commented Oct 3, 2018

@jglick PersistenceProblemsTests should be taken very seriously indeed. For me, if I use workflow-job 2.25 and everything else the same, these tests pass. And unfortunately they still fail with my AsynchronousExecution fix applied (so far), which suggests something deeper is amiss.

@jglick
Copy link
Member Author

jglick commented Oct 3, 2018

if I use workflow-job 2.25 and everything else the same, these tests pass

Not for me, at least not quite. See #252 (comment).

they still fail with my AsynchronousExecution fix applied

Passes here after 15f2e7d. Just a bit of flakiness in the tests AFAICT, nothing to do with the upstream PRs except for slight changes in timing.

Copy link
Member

@svanoort svanoort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks reasonable to me, afaict -- would suggest cutting this into a portion that does not depend on the rewrite (so it's independently releasable) and then merging & releasing for workflow-cps.

Copy link
Member

@dwnusbaum dwnusbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fixes here look reasonable to me.

@svanoort
Copy link
Member

svanoort commented Oct 4, 2018

(Note only reason I'm not approving is because it needs to be converted to something backported for merge there)

@jglick jglick changed the title [JEP-210] Integration testing and minor follow-up fixes [JEP-210] Integration testing Oct 4, 2018
@jglick
Copy link
Member Author

jglick commented Oct 4, 2018

At this point it may as well be merged since it is already picking up some parts of JEP-210.

@jglick
Copy link
Member Author

jglick commented Oct 4, 2018

(and it is only a test-scoped dep anyway)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants