Skip to content

Conversation

@timja
Copy link
Member

@timja timja commented Jan 29, 2022

No description provided.

@timja
Copy link
Member Author

timja commented Jan 29, 2022

➜  plugin-compat-tester git:(162dffc) git bisect good
ae2dd7130a56f7298437cd0a72cd7d4a06eb3fe3 is the first bad commit
commit ae2dd7130a56f7298437cd0a72cd7d4a06eb3fe3
Author: imonteroperez <[email protected]>
Date:   Wed Sep 8 11:51:51 2021 +0200

    Bugfix on dealing with non-executed test phase due collateral issues

 .../org/jenkins/tools/test/PluginCompatTester.java | 13 ++++++----
 .../tools/test/util/ExecutedTestNamesDetails.java  | 29 ++++++++++++++++------
 .../tools/test/util/ExecutedTestNamesSolver.java   | 22 ++++++++--------
 .../jenkins/tools/test/PluginCompatTesterTest.java | 24 ++++++++++++++++++
 .../src/test/resources/bad-surefire-exclusion-list |  1 +
 5 files changed, 67 insertions(+), 22 deletions(-)
 create mode 100644 plugins-compat-tester/src/test/resources/bad-surefire-exclusion-list

@timja
Copy link
Member Author

timja commented Jan 29, 2022

According to https://ci.jenkins.io/job/Tools/job/bom/job/PR-863/2/pipeline-console/?selected-node=332

The build is erroring when injected tests fail

@jetersen
Copy link
Member

Perhaps the injected tests do not add value to PCT?

@timja
Copy link
Member Author

timja commented Jan 29, 2022

Perhaps the injected tests do not add value to PCT?

Yeah possibly, we already ignore them in bom

bom/pct.sh

Line 41 in b9fa01a

if [ -f $t/test-classes/InjectedTest.class -a \! -f $t/surefire-reports/TEST-InjectedTest.xml ]

I'm reverting the PR affecting us in our fork of PCT anyway jenkinsci/plugin-compat-tester#343

Upgrading should be dealt with separately to fixing the jep-229 issue imo

@timja timja marked this pull request as ready for review January 30, 2022 17:36
@timja
Copy link
Member Author

timja commented Jan 30, 2022

I manually deployed a SNAPSHOT because the folder is setup to merge with master bringing in changes that cause bom to fail.

I tried reverting the PR that caused issues but was having difficulty.

this could work in the meantime what do people think?

@jetersen
Copy link
Member

Whatever works but could you get a link to a commit sha at least of what is in the snapshot? 🤔

Would be nice to finally land a proper PCT fix?

@timja
Copy link
Member Author

timja commented Jan 30, 2022

done

@timja timja requested a review from jglick January 31, 2022 16:45
prep.sh Outdated
version=0.5.1-rc1066.9cfd30140d30 # TODO https://github.com/jenkinsci/plugin-compat-tester/pull/276
pct=$HOME/.m2/repository/org/jenkins-ci/tests/plugins-compat-tester-cli/${version}/plugins-compat-tester-cli-${version}.jar
[ -f $pct ] || $MVN dependency:get -Dartifact=org.jenkins-ci.tests:plugins-compat-tester-cli:${version}:jar -DremoteRepositories=https://repo.jenkins-ci.org/public/,https://repo.jenkins-ci.org/incrementals/ -Dtransitive=false
version=0.5.1-SNAPSHOT # TODO https://github.com/jenkinsci/plugin-compat-tester/pull/344 (c9c684eab7e8de91650ad1c96ded6489c42910f1)
Copy link
Member

Choose a reason for hiding this comment

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

Better to get jenkinsci/plugin-compat-tester#344 to pass tests—by ignoring stuff if necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

ha, my last attempt of just reverting the PCT change that affected bom didn't go so well as there were other changes. Unless there's a simpler fix I'd probably just revert every PR in the last year =/.

pr-merge pain -.-, can't build an isolated PR

Not sure if I'll have the time to fix properly.

Copy link
Member

Choose a reason for hiding this comment

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

Ack. PCT needs a lot of work.

@jglick jglick marked this pull request as draft January 31, 2022 16:50
@jglick
Copy link
Member

jglick commented Jan 31, 2022

See #657. Ought to remain in draft until it is picking up an incremental build (if not an actual release).

timestamp=0.5.1-20220130.152941-4
pct=$HOME/.m2/repository/org/jenkins-ci/tests/plugins-compat-tester-cli/${version}/plugins-compat-tester-cli-${timestamp}.jar
[ -f $pct ] || $MVN dependency:get -Dartifact=org.jenkins-ci.tests:plugins-compat-tester-cli:${timestamp}:jar -DremoteRepositories=https://repo.jenkins-ci.org/public/,https://repo.jenkins-ci.org/incrementals/ -Dtransitive=false
version=1152.vafc19b26d5aa # TODO https://github.com/jenkinsci/plugin-compat-tester/pull/345
Copy link
Member

Choose a reason for hiding this comment

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

@timja timja marked this pull request as ready for review January 31, 2022 21:40
@timja timja requested a review from jglick January 31, 2022 22:15
@timja timja merged commit b42a3d6 into jenkinsci:master Feb 1, 2022
@timja timja deleted the try-update-bom branch February 1, 2022 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants