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

Integration testing utilities for Jenkins plugins #2467

Closed
uhafner opened this issue Mar 27, 2022 · 15 comments
Closed

Integration testing utilities for Jenkins plugins #2467

uhafner opened this issue Mar 27, 2022 · 15 comments
Labels
hosting-request Request to host a component in jenkinsci

Comments

@uhafner
Copy link
Contributor

uhafner commented Mar 27, 2022

Repository URL

https://github.com/uhafner/plugin-testutil

New Repository Name

lib-plugin-testutil

Description

Integration testing utilities for Jenkins plugins: this library contains all the testing utilities of https://github.com/jenkinsci/plugin-util-api-plugin so that they can be used easier for other plugins. Since the test-jar of the plugin-util-api-plugin does not declare its transitive dependencies, it makes sense to provide a new library that contains all those utilities that correctly provides the required transitive dependencies to run the tests.

The following features are provided:

  • JUnit 5 rule support (see JENKINS-48466): you can now write unit and integration tests with JUnit 5 only. No need to fall back to outdated JUnit 4 anymore.
  • Integration test base classes with a lot of helper methods (for integration tests with a Jenkins instance per either test suite or test class).
  • Architecture rules for ArchUnit that can be used to check for typical Jenkins design problems.
  • Docker Agent setup for integration tests: simply run your tests on a Docker based agent to verify the remoting behavior of a plugin.

See jenkinsci/plugin-util-api-plugin#161 and the downstream PRs for the reasoning of this library.

GitHub users to have commit permission

@uhafner

Jenkins project users to have release permission

drulli

Issue tracker

Jira

@uhafner uhafner added the hosting-request Request to host a component in jenkinsci label Mar 27, 2022
@github-actions
Copy link

Hello from your friendly Jenkins Hosting Checker

It appears you have some issues with your hosting request. Please see the list below and correct all issues marked Required. Your hosting request will not be approved until these issues are corrected. Issues marked with Warning or Info are just recommendations and will not stall the hosting process.

  • ⛔ Required: The groupId for your parent pom is not "org.jenkins-ci.plugins," if this is not a plugin hosting request, you can disregard this notice.
  • ⛔ Required: The 'artifactId' from the pom.xml (plugin-testutil) is incorrect, it should be lib-testutil ('New Repository Name' field with "-plugin" removed)
  • ⛔ Required: 'New Repository Name' must end with "-plugin" (disregard if you are not requesting hosting of a plugin)

You can re-trigger a check by editing your hosting request or by commenting /hosting re-check

@timja
Copy link
Member

timja commented Mar 28, 2022

@jglick @basil @jtnord @Vlatombe any thoughts?

Looks good to me though :)

@jglick
Copy link
Contributor

jglick commented Mar 28, 2022

https://github.com/uhafner/plugin-testutil/blob/30c9cfc49b8888d6a0769621c89a60627ea17f73/pom.xml#L85-L96 + https://github.com/uhafner/plugin-testutil/blob/30c9cfc49b8888d6a0769621c89a60627ea17f73/pom.xml#L149-L161 makes me nervous; splitting the test harness for configuration-as-code into its own module caused us big headaches in the past, still not fully resolved (jenkinsci/plugin-compat-tester#276). Recommend verifying that any actual uses of this library behave themselves when run from PCT.

@timja
Copy link
Member

timja commented Mar 28, 2022

That could be checked via a bom incremental once this is hosted 👍

@jglick
Copy link
Contributor

jglick commented Mar 28, 2022

Possibly yeah. IIRC the problem was trickier—that the version of the test harness was not updated in tandem with the version of the plugin. I suppose that would not be an issue in this case because they live in separate repositories with independent version numbers. Just be really careful not to run afoul of the fragile logic in https://github.com/jenkinsci/maven-hpi-plugin/blob/9e82b7e370cc924f89f2748c76b1fda14267c2d3/src/main/java/org/jenkinsci/maven/plugins/hpi/HplMojo.java#L128-L139 + https://github.com/jenkinsci/maven-hpi-plugin/blob/9e82b7e370cc924f89f2748c76b1fda14267c2d3/src/main/java/org/jenkinsci/maven/plugins/hpi/AbstractHpiMojo.java#L516-L520. (Thinking about requiring plugins to enumerate a list of groupId:artifactIds they expect to bundle in WEB-INF/lib/*.jar to avoid surprises. Would be incompatible though.)

@timja timja removed the needs-fix label Mar 28, 2022
@basil
Copy link
Contributor

basil commented Mar 28, 2022

Is there a particular reason why this is a separate project rather than being added to the Jenkins test harness? I am a bit confused about why we need a separate project here. What exactly would be the criteria for adding something to this project as opposed to the official test harness? I think it would be preferable to avoid a repeat of e.g. workflow-basic-steps vs pipeline-utility-steps. Jenkins already has too many competing repositories. Can we combine our efforts and work together on the same code?

(If this is just a matter of granting Dr Hafner commit access to jenkins-test-harness, I am sure that could be arranged. And if this is a matter of efficiency (i.e., being able to iterate quickly without being forced to submit to extended code reviews of questionable value), I would be happy to consider solutions. Whatever the reasons may be, I think it would be preferable to work together on this in the main Jenkins test harness project if possible.)

@uhafner
Copy link
Contributor Author

uhafner commented Mar 28, 2022

My main motivation was that using JUnit 4 and 5 both in jenkins-test-harness did not work (see https://issues.jenkins.io/browse/JENKINS-48466 and jenkinsci/jenkins-test-harness#381 and jenkinsci/jenkins-test-harness#303). So I thought it would be a good start to see if it works outside of the test harness so that plugins already can use it without waiting for a fix. Since it works outside now I thought it make sense to have a separate library for it (see jenkinsci/plugin-util-api-plugin#161).

@basil
Copy link
Contributor

basil commented Mar 28, 2022

My main motivation was that using JUnit 4 and 5 both in jenkins-test-harness did not work (see https://issues.jenkins.io/browse/JENKINS-48466 and jenkinsci/jenkins-test-harness#381 and jenkinsci/jenkins-test-harness#303).

It is not so much that previous efforts did not work at all, but rather that previous efforts resulted in subtle changes in behavior that exposed pre-existing issues in the core test suite that required further investigation and debugging. That is part of the challenge of upgrading old libraries, a challenge with which I am very familiar (having done many such library upgrades, in the Jenkins project and elsewhere).

So I thought it would be a good start to see if it works outside of the test harness so that plugins already can use it without waiting for a fix.

I think that solution makes sense in the short term, but we also have to think of the long-term health of the project as well. By exposing a new API and encouraging others to start using it, we take on technical debt. Then, when the long-term fix is made to the official Jenkins test harness, this technical debt will need to be unraveled: the code that was changed to depend on plugin-testutil will need to be adapted to depend on the hypothetical future JUnit 5-supporting jenkins-test-harness. It is possible that such efforts may stall and we may be stuck with supporting two APIs forever. That would not be a desirable outcome, in my opinion.

I think it would be better to focus our efforts to work together toward the completion of what was started in jenkins-test-harness. Yes, debugging the core test suite issues will be painful, but if we all work together on it, I think it could be done and would result in a better long-term outcome. I am happy to participate in this effort by lending analytical insights and focusing the direction of the investigation, but I cannot dedicate the time to be the primary investigator.

@jglick
Copy link
Contributor

jglick commented Mar 28, 2022

I agree that most of what is in plugin-testutil looks like it should become part of jenkins-test-harness sooner or later, assuming it is sufficiently general, so that should be borne in mind. It seems harmless enough to offer this repository as a sort of playground for APIs that might eventually migrate to JTH—one at a time, in PRs demonstrating their expected use case, so there is a place to discuss API design issues.

My immediate concern is about the plugin deps, which are best avoided in test harness artifacts due to technical difficulties with Maven. I think you could do without. For example, the Pipeline deps seem to support just a small number of utilities related to defining jobs, which could be inlined, or if necessary proposed for addition to the tests JARs of some workflow-* plugins. The ssh-slaves dep could perhaps be removed by simplifying how Dockerized agents are launched: do the equivalent of https://github.com/jenkinsci/docker-agent#usage using SimpleCommandLauncher, or use an inbound agent as in https://github.com/jenkinsci/workflow-cps-plugin/blob/d0a8f6a1c2631aecac324ba574a22d4a55425463/src/test/java/org/jenkinsci/plugins/workflow/cps/SerialFormTest.java#L79-L86.

@basil
Copy link
Contributor

basil commented Mar 28, 2022

It seems harmless enough to offer this repository as a sort of playground for APIs that might eventually migrate to JTH—one at a time, in PRs demonstrating their expected use case, so there is a place to discuss API design issues.

Harmless enough only to the extent that plugins do not become dependent on these experimental APIs and force us to maintain these experimental APIs in the long term or laboriously migrate away from them. If the present author explicitly declares an intention to drive these APIs to completion in jenkins-test-harness and migrate callers from the experimental APIs to the official ones in the process, then this would mitigate my concerns. In the absence of such a commitment, then these APIs could very well become technical debt, and migrating off of them may very well fall on build maintainers: people like me who would rather avoid such chores if we can help it.

@uhafner
Copy link
Contributor Author

uhafner commented Mar 28, 2022

I see. I am fine with not publishing this as a separate library.

So how should I proceed to use these JUnit 5 rules in my plugins then? Leaving them in the tests folder of https://github.com/jenkinsci/plugin-util-api-plugin (see jenkinsci/plugin-util-api-plugin#161)? Or will this cause the same problems?

I simply do not want to copy the same things again and again for several of my plugins.

@basil
Copy link
Contributor

basil commented Mar 29, 2022

I see. I am fine with not publishing this as a separate library.

Well, in my previous post I invited you to work together with us in jenkins-test-harness to upstream these valuable contributions in a way that can be consumed throughout the ecosystem (including in Jenkins core), and while you have not responded in the negative about this, you also have not responded in the positive either, so I am not sure what to make of things. I will not press you further about this.

I simply do not want to copy the same things again and again for several of my plugins.

If you simply want to deduplicate functionality within your plugins and have no interest in generalizing it for the broader Jenkins community, I think that would be acceptable but disappointing, and would not block this request.

@uhafner
Copy link
Contributor Author

uhafner commented Mar 30, 2022

I see. I am fine with not publishing this as a separate library.

Well, in my previous post I invited you to work together with us in jenkins-test-harness to upstream these valuable contributions in a way that can be consumed throughout the ecosystem (including in Jenkins core), and while you have not responded in the negative about this, you also have not responded in the positive either, so I am not sure what to make of things. I will not press you further about this.

I don't think that I have the knowledge and time to find the reason why the JUnit 5 dependencies break the core builds. These PRs have been proposed by several other volunteers (see jenkinsci/jenkins-test-harness#381 and jenkinsci/jenkins-test-harness#303). My goal simply was to make these rules available for plugins until this has been fixed in core.

For the other topics, I am happy to help to improve the test harness (I already did in the past) by providing pull requests.

I simply do not want to copy the same things again and again for several of my plugins.

If you simply want to deduplicate functionality within your plugins and have no interest in generalizing it for the broader Jenkins community, I think that would be acceptable but disappointing, and would not block this request.

Ok. Then I will use them only in my plugins right now (the pull requests are all ready to be merged) and try to make them available for others in a step by step approach.

@uhafner uhafner closed this as completed Mar 30, 2022
@basil
Copy link
Contributor

basil commented Jul 26, 2022

Hi @uhafner, I did the work to reintegrate JUnit 5 support to the Jenkins test harness in jenkinsci/jenkins-test-harness#438, jenkinsci/jenkins#6657, and jenkinsci/plugin-pom#560 which has been successfully integrated as of the current weekly release and plugin parent POM. You can now adapt your plugins to use the official JUnit 5 integration. Let me know if I can be of any assistance.

@uhafner
Copy link
Contributor Author

uhafner commented Jul 27, 2022

Yes, I noticed that. Thanks for finishing this new feature! I'll remove my copy of those extensions soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hosting-request Request to host a component in jenkinsci
Projects
None yet
Development

No branches or pull requests

4 participants