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

Teach PCT about Mina SSHD API plugins #375

Merged
merged 1 commit into from
Jul 26, 2022
Merged

Conversation

basil
Copy link
Member

@basil basil commented Jul 26, 2022

Discovered when running BOM/PCT tests with recent PCT bits in jenkinsci/bom#1059: although current BOM master builds appear fine with regard to the Mina SSHD API plugins, they are using an ancient and nonstandard version of PCT that fails to report many errors. Recent and unmodified PCT bits show errors like the ones from this run (specific log for mina-sshd-api-core) and pct-report.xml contains <status>INTERNAL_ERROR</status>.

This PR corrects the specific problem shown above by teaching PCT about the Mina SSHD API plugins, and I tested it in this run (see the corresponding build log for mina-sshd-api-core). The build progresses further than before (as demonstrated by comparing the two build logs for mina-sshd-api-core), although ultimately the tests for mina-sshd-api-core still do not get executed: unlike e.g. pipeline-stage-view (which triggers this code path, which works), the Mina SSHD API plugins are in a multi-module Maven project with JEP-229 enabled (which triggers this code path, skipping all tests because they are apparently not supported in multimodule Maven projects with JEP-229 enabled). I filed any latent curiousity about this gap in feature parity between JEP-229 and non-JEP-229 plugins for another day and decided to proceed with the current PR which at least improves the status quo. Besides, this PR is completely risk-free for anything other than the Mina SSHD API plugins, so merging it cannot hurt. And most importantly, it is the last thing standing in my way to completing jenkinsci/bom#1059.

@basil basil marked this pull request as ready for review July 26, 2022 20:08
@basil basil requested a review from a team as a code owner July 26, 2022 20:08
@basil
Copy link
Member Author

basil commented Jul 26, 2022

/reviewer @jenkinsci/teams/plugin-compat-tester-developers

@basil
Copy link
Member Author

basil commented Jul 26, 2022

/label enhancement

@basil
Copy link
Member Author

basil commented Jul 26, 2022

/reviewer @jenkinsci/plugin-compat-tester-developers

@jglick
Copy link
Member

jglick commented Jul 26, 2022

The bot has been turned off on most repos for now IIUC.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Design flaw in PCT that it even requires custom hooks for the routine case of a multimodule project, but in the meantime, we need this I guess.

@basil
Copy link
Member Author

basil commented Jul 26, 2022

Design flaw in PCT

One of many. 😀

@jglick
Copy link
Member

jglick commented Jul 26, 2022

they are apparently not supported in multimodule Maven projects with JEP-229 enabled

Not familiar with the thing you are discussing unfortunately. I recall a problem being worked around in #318.

@jglick jglick enabled auto-merge July 26, 2022 20:17
@basil
Copy link
Member Author

basil commented Jul 26, 2022

Not familiar with the thing you are discussing unfortunately.

Well I linked to the code. I am even less familiar than you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants