use a more simple Jenkinsfile and build on windows #508

olamy · 2022-02-23T01:45:28Z

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue

Jenkinsfile

jglick

This is archiving bogus test results like https://ci.jenkins.io/job/Tools/job/plugin-pom/job/PR-508/5/testReport/io.jenkins.plugins/RandomClassTest/linux_8___Build__linux_8____whatever/. (Whereas ITs are not tracked.) Can we at least suppress junit collecting TEST-*.xml from IT subdirs?

jglick · 2022-02-23T15:49:42Z

Jenkinsfile

-        ansiColor('xterm') {
-            withEnv(['MAVEN_OPTS=-Djansi.force=true']) {
-                sh 'mvn -B -Dstyle.color=always -ntp clean verify'


Results in much less readable IT logs unfortunately.

Results in much less readable IT logs unfortunately.

do you mean in Jenkins?

because before it looks a bit strange to me and full of weird characters (from https://ci.jenkins.io/job/Tools/job/plugin-pom/job/master/601/console )

whereas with this change (https://ci.jenkins.io/job/Tools/job/plugin-pom/job/PR-508/5/console)

I am afraid the ansicolor plugin is buggy when it comes to partial logs. https://ci.jenkins.io/job/Tools/job/plugin-pom/job/master/601/consoleFull looks nice.

see jenkins-infra/pipeline-library#263

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>

Signed-off-by: Olivier Lamy <olamy@apache.org>

olamy · 2022-02-24T02:58:57Z

weird pipeline cannot find invoker junit results whereas https://github.com/jenkins-infra/pipeline-library/blob/4688c7b4d73ea5450863c99ea56df544a904928b/vars/buildPlugin.groovy#L118

Signed-off-by: Olivier Lamy <olamy@apache.org>

olamy · 2022-02-24T06:19:25Z

arff need the boolean. and need this as well jenkins-infra/pipeline-library#307

jglick · 2022-02-24T15:00:13Z

I suspect jenkins-infra/pipeline-library#307 is missing the point. We want for example target/invoker-reports/BUILD-undefined-java-level.xml. (If the junit step can process this format, and we make sure buildPlugin sets the appropriate flag to the Invoker plugin to not fail the build when there are test failures.) The junk here is stuff like target/its/sample-plugin/target/surefire-reports/TEST-test.SampleRootActionTest.xml which are not test results from plugin-pom but test results from a plugin being built in an IT (and some ITs could in fact assert that a plugin has test failures).

jglick · 2022-02-24T15:08:52Z

In general, I am not really a fan of using the buildPlugin function to build things which are not plugins. On the one hand buildPlugin includes lots of logic which is not appropriate for a POM project; on the other hand, it misses some things which are. Seems easier and safer to either continue using a dedicated Jenkinsfile, just extending it to run Windows builds; or introduce some new function to build generic Maven projects with @jenkinsci-specific logic (such as JEP-305 incremental deployment), as inhttps://github.com/jenkins-infra/pipeline-library/pull/56.

olamy · 2022-02-24T20:52:37Z

I suspect jenkins-infra/pipeline-library#307 is missing the point. We want for example target/invoker-reports/BUILD-undefined-java-level.xml. (If the junit step can process this format, and we make sure buildPlugin sets the appropriate flag to the Invoker plugin to not fail the build when there are test failures.) The junk here is stuff like target/its/sample-plugin/target/surefire-reports/TEST-test.SampleRootActionTest.xml which are not test results from plugin-pom but test results from a plugin being built in an IT (and some ITs could in fact assert that a plugin has test failures).

please note this commit 1ca45c0 which prevents maven IT tests to generate some surefire reports

jglick · 2022-02-24T21:29:28Z

Right. Artificially changing the behavior of a plugin build just so that we do not accidentally pick up files which we should not have been looking for to begin with does not seem ideal, though.

olamy · 2022-02-25T00:21:18Z

Right. Artificially changing the behavior of a plugin build just so that we do not accidentally pick up files which we should not have been looking for to begin with does not seem ideal, though.

Sorry I don't understand which files you are referring too? Can you please explain? thanks
anyway there are some interesting issues with bad characters including in the logs coming from the build. https://ci.jenkins.io/blue/organizations/jenkins/Tools%2Fplugin-pom/detail/PR-508/8/tests/
still something interesting to figure out.

… streaming logs only on failure might be a more interesting option to avoid noise Signed-off-by: Olivier Lamy <olamy@apache.org>

Signed-off-by: Olivier Lamy <olamy@apache.org>

…rded Signed-off-by: Olivier Lamy <olamy@apache.org>

olamy · 2022-02-25T03:21:44Z

ok I figure out. now build logs from ITs are recorded separately and easier to find than looking at the full console output.

jglick · 2022-02-25T14:10:30Z

I don't understand which files you are referring to?

target/its/sample-plugin/target/surefire-reports/TEST-test.SampleRootActionTest.xml for example. There is nothing wrong with such files being created as part of the sample plugin build process, and it distorts the IT to suppress them. The only reason to do so is to work around the behavior of buildPlugin in looking for such files, which it does because it is expecting to be run on a plugin, not on something which runs ITs.

jglick

IT test results are now being recorded by junit, which seems to work well. (Do not have an example of a failing test but I suppose this would include the mvn subcommand output in the test stdio.)

Another problem caused by inappropriately using a script intended for a plugin build on something that runs ITs of plugins: https://ci.jenkins.io/job/Tools/job/plugin-pom/job/PR-508/12/artifact/target/its/ archiving lots of junk.

Still -0 on this I guess. Works, but more awkwardly than directly writing a Jenkinsfile tailored to the details of the repository.

jglick · 2022-02-25T14:11:02Z

pom.xml

-              <streamLogs>true</streamLogs>
+              <writeJunitReport>true</writeJunitReport>
+              <junitPackageName>io.jenkins.plugins.pom.its</junitPackageName>
+<!--              <streamLogs>true</streamLogs>-->


Suggested change

jglick · 2022-02-25T14:11:27Z

src/it/benchmark/invoker.properties

@@ -1 +1 @@
-invoker.goals=-P jmh-benchmark -Dstyle.color=always -ntp clean test
+invoker.goals=-P jmh-benchmark  -ntp clean test


Suggested change

invoker.goals=-P jmh-benchmark -ntp clean test

invoker.goals=-P jmh-benchmark -ntp clean test

jglick · 2022-02-25T14:11:40Z

src/it/beta-pass/invoker.properties

@@ -1 +1 @@
-invoker.goals=-Dstyle.color=always -ntp clean install
+invoker.goals= -ntp clean install


Suggested change

invoker.goals= -ntp clean install

invoker.goals=-ntp clean install

jglick · 2022-02-25T14:11:48Z

src/it/incrementals-and-plugin-bom/invoker.properties

@@ -1,3 +1,3 @@
-invoker.goals.1=-Dstyle.color=always -ntp clean install
+invoker.goals.1= -ntp clean install


Suggested change

invoker.goals.1= -ntp clean install

invoker.goals.1=-ntp clean install

jglick · 2022-02-25T14:11:57Z

src/it/incrementals-and-plugin-bom/invoker.properties

 # real extension will not work here due to its not being at the root of a repository, so fake it:
-invoker.goals.2=-Dstyle.color=always -ntp -Dset.changelist -Dchangelist=-rc1234.deadbeef5678 clean install
+invoker.goals.2= -ntp -Dset.changelist -Dchangelist=-rc1234.deadbeef5678 clean install


Suggested change

invoker.goals.2= -ntp -Dset.changelist -Dchangelist=-rc1234.deadbeef5678 clean install

invoker.goals.2=-ntp -Dset.changelist -Dchangelist=-rc1234.deadbeef5678 clean install

jglick · 2022-02-25T14:12:05Z

src/it/localizer/invoker.properties

@@ -1,2 +1,2 @@
 # release.skipTests normally set in jenkins-release profile since release:perform would do the tests
-invoker.goals=-Dstyle.color=always -ntp -Pjenkins-release -Drelease.skipTests=false clean verify
+invoker.goals= -ntp -Pjenkins-release -Drelease.skipTests=false clean verify


Suggested change

invoker.goals= -ntp -Pjenkins-release -Drelease.skipTests=false clean verify

invoker.goals=-ntp -Pjenkins-release -Drelease.skipTests=false clean verify

jglick · 2022-02-25T14:12:12Z

src/it/sample-plugin/invoker.properties

@@ -1,3 +1,3 @@
 # install, not verify, because we want to check the artifact as we would be about to deploy it
 # release.skipTests normally set in jenkins-release profile since release:perform would do the tests
-invoker.goals=-Dstyle.color=always -ntp -Pjenkins-release -Drelease.skipTests=false clean install
+invoker.goals= -ntp -Pjenkins-release -Drelease.skipTests=false clean install


Suggested change

invoker.goals= -ntp -Pjenkins-release -Drelease.skipTests=false clean install

invoker.goals=-ntp -Pjenkins-release -Drelease.skipTests=false clean install

jglick · 2022-02-25T14:12:26Z

src/it/settings.xml

@@ -36,6 +36,9 @@ under the License.
      <activation>
        <activeByDefault>true</activeByDefault>
      </activation>
+      <properties>
+        <disableXmlReport>true</disableXmlReport>


This is the part that makes me somewhat uncomfortable.

well you wanted to avoid noise (#508 (review)) from those files which should not be scanned by buildPlugin so I tried to address this in 2 ways: this and this one jenkins-infra/pipeline-library#307
and this would have impact only if we have an IT test running mvn site and making some assertions on having the surefire-report html file correctly generated.
surefire does not use those files to detect error there are only here to build reports by other plugins.

surefire does not use those files to detect error there are only here to build reports by other plugins.

Yes, I know.

jglick · 2022-02-25T14:12:35Z

src/it/undefined-java-level/invoker.properties

@@ -1,2 +1,2 @@
-invoker.goals=-Dstyle.color=always -ntp clean install
+invoker.goals= -ntp clean install


Suggested change

invoker.goals= -ntp clean install

invoker.goals=-ntp clean install

jglick · 2022-02-25T14:48:17Z

pom.xml

+              <writeJunitReport>true</writeJunitReport>
+              <junitPackageName>io.jenkins.plugins.pom.its</junitPackageName>
+<!--              <streamLogs>true</streamLogs>-->
+              <streamLogsOnFailures>true</streamLogsOnFailures>


I think you do not actually want this. If a test fails, junit should record the failure with details right?

Suggested change

<streamLogsOnFailures>true</streamLogsOnFailures>

just to have the failure directly in the build output in case of IT failure which may be a bit more convenient when running test locally.
see output here https://ci.jenkins.io/job/Tools/job/plugin-pom/job/PR-508/11/console

when running test locally

OK. Or you can use the profile trick I developed in jenkinsci/maven-hpi-plugin#301.

olamy · 2022-02-25T22:10:24Z

I don't understand which files you are referring to?

target/its/sample-plugin/target/surefire-reports/TEST-test.SampleRootActionTest.xml for example. There is nothing wrong with such files being created as part of the sample plugin build process, and it distorts the IT to suppress them. The only reason to do so is to work around the behavior of buildPlugin in looking for such files, which it does because it is expecting to be run on a plugin, not on something which runs ITs.

buildPlugin should not do such deep scan as surefire just write test results in target/surefire-reports/
technically I do not see any reason to have such deep scanning.

IT test results are now being recorded by junit, which seems to work well. (Do not have an example of a failing test but I suppose this would include the mvn subcommand output in the test stdio.)

Another problem caused by inappropriately using a script intended for a plugin build on something that runs ITs of plugins: https://ci.jenkins.io/job/Tools/job/plugin-pom/job/PR-508/12/artifact/target/its/ archiving lots of junk.

Still -0 on this I guess. Works, but more awkwardly than directly writing a Jenkinsfile tailored to the details of the repository.

if you are -0 that's fine we can forget this.
We spend too much time on it and it looks to turn into a dead end and waste both of our time.
It's weekend I definitely have better waves to surf (aussie version) 🏄

jglick · 2022-02-25T22:14:44Z

technically I do not see any reason to have such deep scanning.

Yeah it is hard to say. target/surefire-reports/ is the normal. */target/surefire-reports/ is pretty common. Is */*/target/surefire-reports/ ever used? Or anything deeper? Not sure. Maybe we could get away with scanning for test results only at top level or one layer deep.

olamy marked this pull request as ready for review February 23, 2022 02:10

timja reviewed Feb 23, 2022

View reviewed changes

Jenkinsfile Outdated Show resolved Hide resolved

timja approved these changes Feb 23, 2022

View reviewed changes

timja added the chore label Feb 23, 2022

jglick reviewed Feb 23, 2022

View reviewed changes

olamy and others added 6 commits February 24, 2022 12:32

use a more simple Jenkinsfile and build on windows

d521444

oops typo

5c35922

quick test to ensure this would have failed

6f05328

sadly this doesn't break anything (#506)

cdc0653

Update Jenkinsfile

6959c5b

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>

disable surefire xml generation of ITs as junit is going too deep

1ca45c0

Signed-off-by: Olivier Lamy <olamy@apache.org>

olamy force-pushed the simple-jenkinsfile-with-windows-build branch from 94bbbd5 to 1ca45c0 Compare February 24, 2022 02:32

fix invoker reports junit file generation

ea47b5c

Signed-off-by: Olivier Lamy <olamy@apache.org>

timja mentioned this pull request Feb 24, 2022

avoid deep pattern for maven-invoker-plugin generated junit files jenkins-infra/pipeline-library#307

Closed

olamy added 4 commits February 25, 2022 10:24

streaming logs might generate some weird characters because of jansi,…

fa67727

… streaming logs only on failure might be a more interesting option to avoid noise Signed-off-by: Olivier Lamy <olamy@apache.org>

this one might be the guilty, have a test

c36f2fc

Signed-off-by: Olivier Lamy <olamy@apache.org>

more testing

ae019a2

Signed-off-by: Olivier Lamy <olamy@apache.org>

revert IT failure to ensure It logs are in console and correctly reco…

e8b3acc

…rded Signed-off-by: Olivier Lamy <olamy@apache.org>

jglick reviewed Feb 25, 2022

View reviewed changes

jglick mentioned this pull request Feb 25, 2022

Record maven-invoker-plugin failures via junit rather than console jenkinsci/maven-hpi-plugin#301

Merged

olamy closed this Feb 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use a more simple Jenkinsfile and build on windows #508

use a more simple Jenkinsfile and build on windows #508

olamy commented Feb 23, 2022

jglick left a comment

jglick Feb 23, 2022

olamy Feb 24, 2022 •

edited

Loading

jglick Feb 24, 2022

jglick Feb 24, 2022

olamy commented Feb 24, 2022

olamy commented Feb 24, 2022

jglick commented Feb 24, 2022

jglick commented Feb 24, 2022

olamy commented Feb 24, 2022

jglick commented Feb 24, 2022

olamy commented Feb 25, 2022

olamy commented Feb 25, 2022

jglick commented Feb 25, 2022

jglick left a comment

jglick Feb 25, 2022

jglick Feb 25, 2022

jglick Feb 25, 2022

jglick Feb 25, 2022

jglick Feb 25, 2022

jglick Feb 25, 2022

jglick Feb 25, 2022

jglick Feb 25, 2022

olamy Feb 25, 2022

jglick Feb 25, 2022

jglick Feb 25, 2022

jglick Feb 25, 2022

olamy Feb 25, 2022

jglick Feb 25, 2022

olamy commented Feb 25, 2022

jglick commented Feb 25, 2022

		@@ -1 +1 @@
		invoker.goals=-P jmh-benchmark -Dstyle.color=always -ntp clean test
		invoker.goals=-P jmh-benchmark -ntp clean test

		@@ -1 +1 @@
		invoker.goals=-Dstyle.color=always -ntp clean install
		invoker.goals= -ntp clean install

	invoker.goals= -ntp clean install
	invoker.goals=-ntp clean install

		@@ -1,3 +1,3 @@
		invoker.goals.1=-Dstyle.color=always -ntp clean install
		invoker.goals.1= -ntp clean install

	invoker.goals.1= -ntp clean install
	invoker.goals.1=-ntp clean install

	invoker.goals.2= -ntp -Dset.changelist -Dchangelist=-rc1234.deadbeef5678 clean install
	invoker.goals.2=-ntp -Dset.changelist -Dchangelist=-rc1234.deadbeef5678 clean install

	invoker.goals= -ntp -Pjenkins-release -Drelease.skipTests=false clean verify
	invoker.goals=-ntp -Pjenkins-release -Drelease.skipTests=false clean verify

		@@ -1,2 +1,2 @@
		invoker.goals=-Dstyle.color=always -ntp clean install
		invoker.goals= -ntp clean install

use a more simple Jenkinsfile and build on windows #508

use a more simple Jenkinsfile and build on windows #508

Conversation

olamy commented Feb 23, 2022

jglick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olamy Feb 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olamy commented Feb 24, 2022

olamy commented Feb 24, 2022

jglick commented Feb 24, 2022

jglick commented Feb 24, 2022

olamy commented Feb 24, 2022

jglick commented Feb 24, 2022

olamy commented Feb 25, 2022

olamy commented Feb 25, 2022

jglick commented Feb 25, 2022

jglick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olamy commented Feb 25, 2022

jglick commented Feb 25, 2022

olamy Feb 24, 2022 •

edited

Loading