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.

Sign up for GitHub

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

use a more simple Jenkinsfile and build on windows #508

Closed
wants to merge 11 commits into from
Closed

use a more simple Jenkinsfile and build on windows #508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d521444
use a more simple Jenkinsfile and build on windows
olamy Feb 23, 2022
5c35922
oops typo
olamy Feb 23, 2022
6f05328
quick test to ensure this would have failed
olamy Feb 23, 2022
cdc0653
sadly this doesn't break anything (https://github.com/jenkinsci/plug…
olamy Feb 23, 2022
6959c5b
Update Jenkinsfile
olamy Feb 23, 2022
1ca45c0
disable surefire xml generation of ITs as junit is going too deep
olamy Feb 24, 2022
ea47b5c
fix invoker reports junit file generation
olamy Feb 24, 2022
fa67727
streaming logs might generate some weird characters because of jansi,…
olamy Feb 25, 2022
c36f2fc
this one might be the guilty, have a test
olamy Feb 25, 2022
ae019a2
more testing
olamy Feb 25, 2022
e8b3acc
revert IT failure to ensure It logs are in console and correctly reco…
olamy Feb 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
properties([buildDiscarder(logRotator(numToKeepStr: '20'))])
node('maven-11') {
checkout scm
timeout(time: 1, unit: 'HOURS') {
// TODO Azure mirror
ansiColor('xterm') {
withEnv(['MAVEN_OPTS=-Djansi.force=true']) {
sh 'mvn -B -Dstyle.color=always -ntp clean verify'
Comment on lines -6 to -8
Copy link
Member

Choose a reason for hiding this comment

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

Results in much less readable IT logs unfortunately.

Copy link
Member Author

@olamy olamy Feb 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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 )

Screen Shot 2022-02-24 at 12 10 20 pm

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

Screen Shot 2022-02-24 at 12 11 50 pm

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

see jenkins-infra/pipeline-library#263

}
}
}
}
// Builds a module using https://github.com/jenkins-infra/pipeline-library
buildPlugin(useAci: true, configurations: [
[ platform: "linux", jdk: "8" ],
[ platform: "linux", jdk: "11" ],
[ platform: "windows", jdk: "11" ]
])
5 changes: 4 additions & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,10 @@
<goal>run</goal>
</goals>
<configuration>
<streamLogs>true</streamLogs>
<writeJunitReport>true</writeJunitReport>
<junitPackageName>io.jenkins.plugins.pom.its</junitPackageName>
<!-- <streamLogs>true</streamLogs>-->
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<!-- <streamLogs>true</streamLogs>-->

<streamLogsOnFailures>true</streamLogsOnFailures>
Copy link
Member

Choose a reason for hiding this comment

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

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>

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

when running test locally

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

<showErrors>true</showErrors>
<cloneProjectsTo>${project.build.directory}/its</cloneProjectsTo>
<localRepositoryPath>${basedir}/target/local-repo</localRepositoryPath>
Expand Down
2 changes: 1 addition & 1 deletion src/it/benchmark/invoker.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
invoker.goals=-P jmh-benchmark -Dstyle.color=always -ntp clean test
invoker.goals=-P jmh-benchmark -ntp clean test
Copy link
Member

Choose a reason for hiding this comment

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

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

2 changes: 1 addition & 1 deletion src/it/beta-fail/invoker.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
invoker.goals=-Dstyle.color=always -ntp clean install
invoker.goals=-ntp clean install
invoker.buildResult=failure
2 changes: 1 addition & 1 deletion src/it/beta-just-testing/invoker.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
invoker.goals=-Dstyle.color=always -ntp clean install
invoker.goals=-ntp clean install
2 changes: 1 addition & 1 deletion src/it/beta-pass/invoker.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
invoker.goals=-Dstyle.color=always -ntp clean install
invoker.goals= -ntp clean install
Copy link
Member

Choose a reason for hiding this comment

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

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

4 changes: 2 additions & 2 deletions src/it/incrementals-and-plugin-bom/invoker.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
invoker.goals.1=-Dstyle.color=always -ntp clean install
invoker.goals.1= -ntp clean install
Copy link
Member

Choose a reason for hiding this comment

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

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

# 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
Copy link
Member

Choose a reason for hiding this comment

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

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

2 changes: 1 addition & 1 deletion src/it/localizer/invoker.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

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

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

2 changes: 1 addition & 1 deletion src/it/sample-plugin/invoker.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

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

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

3 changes: 3 additions & 0 deletions src/it/settings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ under the License.
<activation>
<activeByDefault>true</activeByDefault>
</activation>
<properties>
<disableXmlReport>true</disableXmlReport>
Copy link
Member

Choose a reason for hiding this comment

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

This is the part that makes me somewhat uncomfortable.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Yes, I know.

</properties>
<repositories>
<repository>
<id>local.central</id>
Expand Down
2 changes: 1 addition & 1 deletion src/it/undefined-java-level/invoker.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
invoker.goals=-Dstyle.color=always -ntp clean install
invoker.goals= -ntp clean install
Copy link
Member

Choose a reason for hiding this comment

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

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

invoker.buildResult=failure