Standardized & simplified & updated build & test infrastructure #307
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
jglick
changed the title
| @AnalyzeClasses(packages = "io.jenkins.plugins..") | ||
| class ArchitectureTest { | ||
| @ArchTest | ||
| static final ArchRule NO_JENKINS_INSTANCE_CALL = PluginArchitectureRules.NO_JENKINS_INSTANCE_CALL; |
There was a problem hiding this comment.
Whatever this stuff was doing, it can presumably be dropped without anyone noticing.
| */ | ||
| @RunWith(Parameterized.class) | ||
| @SuppressWarnings({"PMD.ExcessiveImports", "checkstyle:ClassDataAbstractionCoupling", "rawtypes", "checkstyle:ClassFanOutComplexity", "checkstyle:JavaNCSS"}) | ||
| public class GitHubChecksPublisherITest extends IntegrationTestWithJenkinsPerTest { |
There was a problem hiding this comment.
This IntegrationTestWithJenkinsPerTest is the problem. Should be rewritten to use just stuff provided by jenkins-test-harness.
| */ | ||
| @Test | ||
| public void shouldRetrieveContextFromPipeline() { | ||
| WorkflowJob job = createPipeline(); |
There was a problem hiding this comment.
Here for example. What is wrong with the usual r.createProject(WorkflowJob.class)?
| import org.jenkinsci.plugins.github_branch_source.GitHubSCMSourceContext; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static io.jenkins.plugins.util.assertions.Assertions.assertThat; |
There was a problem hiding this comment.
A custom version of assertThat?? Just use Hamcrest.
|
@timja any advice here? |
pom.xml
Outdated
| <scope>import</scope> | ||
| <type>pom</type> | ||
| </dependency> | ||
| <dependency> <!-- TODO pending https://github.com/jenkinsci/github-branch-source-plugin/pull/624 --> |
There was a problem hiding this comment.
or is the question more about the deleting of all of the integration tests in the plugin? |
Codecov Report
@@ Coverage Diff @@
## master #307 +/- ##
=============================================
- Coverage 80.79% 50.38% -30.42%
+ Complexity 170 114 -56
=============================================
Files 16 16
Lines 526 526
Branches 49 49
=============================================
- Hits 425 265 -160
- Misses 79 246 +167
+ Partials 22 15 -7 see 10 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Yeah, that is the only thing I found that would work. Do you have a better idea? |
Switch to the default parent pom but leave the |
|
After trying this myself I see that the issue is that plugin-util-api now uses It would be non-trivial to re-write to junit5 I think. I'll give that a quick go and if that's not something I have time for I'll merge this. fyi @uhafner |
|
Shipping to unblock you, can look at the tests separately |
|
Later equals never! 👎 |
|
Why are you requesting a review and then merge such a change before the review happens? |
|
Shouldn't this be done by the author of this PR? |
😄 I've made good progress in here: I found another complexity is that github-branch-source has had some changes which has broken some of the mocking assumptions in this plugin. I've fixed all but one test in that test class and just need to do some debugging to finish it off. Hopefully other tests will be straightforward to port. |
Wanted to try consuming jenkinsci/scm-api-plugin#180 but that first requires having basic dependencies be reasonably up to date, which they are not and the reliance on the
analysis-pomparent makes it far harder to do routine maintenance like we do typical Jenkins plugins. The use of theplugin-util-apitest dependency also causes serious structural problems making it hard to update the POM. (jenkinsci/bom#1521 (comment) for context)Would supersede #306 & #305 & #304 & #302 & #265.