Modifying PR Testing Process

[jupierce]

See openshift/origin#11711 for details. @bparees @gabemontero ptal

[gabemontero]

IGTM ... I'll wait until the origin PR merges before hitting the button.

Thanks!!

[bparees]

do we need to/should we update the PR test job to actually invoke this new image build script instead of whatever it is doing today?

[gabemontero]

We'll minimally need to change the job to set USE_SNAPSHOT_JENKINS_IMAGE=1

Eyeballing the new script's contents, it isn't a necessity to call it from the PR test job. The pros in doing so would be to validate the script. The con would be that PR could be submitted which changes the script in such a way as to mess up the testing.

The con has more weight with me than the pro in this case. But certainly we can discuss more if desired.

[bparees]

The con would be that PR could be submitted which changes the script in such a way as to mess up the testing.

i think that's a pro. having the PR tester actually testing the image build script seems useful.

if your concern is that someone breaks the image build script in such a way that: 1) it still succeeds (so the PR test doesn't fail because it couldn't build the image) 2) it doesn't actually build the image

which results in the PR test not actually testing a newly built image, that's fair, but i think we should just enforcing that a new image gets built successfully as part of the PR testing. I don't like the idea of having an image builder script, but not using it and instead re-implementing that same logic directly in the CI job definition.

[gabemontero]

You captured my concern. I'm afraid the newly built plugin won't get tested because the PR messes up the script that ensures it is tested.

The new script provided feels more like helper to me than something "official". Admittedly subjective distinctions.

I'll look at / consider both approaches when the origin PR merges and we complete the loop here.

[jupierce]

Based on that feedback, I believe I can resolve the concern by making the extended tests blow IF USE_SNAPSHOT_JENKINS_IMAGE=1 but no snapshot image is available.

[gabemontero]

On Tue, Nov 1, 2016 at 4:16 PM, Justin Pierce notifications@github.com wrote:

Based on that feedback, I believe I can resolve the concern by making the extended tests blow IF USE_SNAPSHOT_JENKINS_IMAGE=1 but no snapshot build is available.

—

Not sure I follow what "blow" means here, but I'm guessing it will make sense once you've made the change in origin, and we can go from there. Sounds good.

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openshift/jenkins-plugin/pull/104#issuecomment-257682174, or mute the thread https://github.com/notifications/unsubscribe-auth/ADbadHnK7lQs9G9RaxnDfYdgN3xC0T51ks5q554NgaJpZM4Kmch1 .

[jupierce]

@gabemontero blow = The extended tests will fail if USE_SNAPSHOT_JENKINS_IMAGE is defined but there is no jenkins snapshot image detected. That should detect the scenario where the build script is crippled somehow.

[gabemontero]

@jupierce ok cool thx for the clarification