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

WFLY-18279 Update HostExcludesTestCase configuration to work with WF30 #17038

Merged
merged 2 commits into from Jul 23, 2023

Conversation

pferraro
Copy link
Contributor

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Jul 21, 2023
Copy link
Contributor

@yersan yersan left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 213 to 217
WILDFLY_29_0("WildFly29.0", WILDFLY_28_0, true),
CURRENT(MAJOR, WILDFLY_29_0, getCurrentAddedExtensions(), getCurrentRemovedExtensions(), true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for the record, it is good to have it adjusted now (even if the product version is still 29) but it's not strictly necessary to do it on each new release if we have not added/removed extensions on the current release.

The test won't fail if we have moved the product version to the next one (e.g. 30) but we have not added/removed extensions on the current release (e.g. 29).

Copy link
Contributor Author

@pferraro pferraro Jul 21, 2023

Choose a reason for hiding this comment

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

Without this change, it seems that all full-integration tests on wildfly-core are failing, e.g. https://ci.wildfly.org/viewLog.html?buildId=381306&buildTypeId=WildFlyCore_PullRequest_WildFlyCoreFullIntegrationLinuxJdk11
as well as CI runs for wildfly itself, e.g. https://ci.wildfly.org/viewLog.html?buildId=381292&buildTypeId=WF_PullRequest_LinuxJdk11
Was I mistaken?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test won't fail if we have moved the product version to the next one (e.g. 30) but we have not added/removed extensions on the current release (e.g. 29).

Ah - I see. So, were the product version changed (I assume, via an update to org.wildfly.core:wildfly-version + a wildfly-core upgrade?), this PR would not have been strictly necessary. Correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ups, sorry, I missed this comment and my review was incorrect since there were extensions removed in the current release (WF29).

@bstansberry
Copy link
Contributor

Thanks, @pferraro. Can I punish you for doing this by asking you to update the WF release process doc to include this as an aspect of the "bump the version" release task? This is something I should do as part of the version bump commit but I never remember!

@pferraro
Copy link
Contributor Author

pferraro commented Jul 21, 2023

Thanks, @pferraro. Can I punish you for doing this by asking you to update the WF release process doc to include this as an aspect of the "bump the version" release task? This is something I should do as part of the version bump commit but I never remember!

Sure, no problem.
UPDATE: Done.

@pferraro
Copy link
Contributor Author

@yersan Was I also meant to have moved the extensions referenced within getCurrentRemovedExtensions() to the enum value for WILDFLY_29_0?

@yersan
Copy link
Contributor

yersan commented Jul 21, 2023

@pferraro Yes, right, I missed those extensions. They were removed, so we need to move them as removed extensions for the new enum we are creating. If we don't move them, the test will fail as soon as we moved the major version in the pom.xml: see https://github.com/wildfly/wildfly/pull/17038/files#diff-d4696c308808a57f331264e1d20639f0ef8a33968635726f1d64a3ecb14e93d5L387-L389

This is how the test forces us to keep the enums in sync with what we have ... sadly it creates more headaches than other things

@yersan
Copy link
Contributor

yersan commented Jul 21, 2023

See pferraro#13 these are the additional changes required for this PR

@pferraro
Copy link
Contributor Author

@yersan PR updated to include modules removed in WF29.

@yersan
Copy link
Contributor

yersan commented Jul 21, 2023

Hi @pferraro, a check style issue: "[20:01:33]W: [Step 5/6] [ERROR] /opt/buildAgent/work/e34a6f994de9f7c6/testsuite/domain/src/test/java/org/jboss/as/test/integration/domain/HostExcludesTestCase.java:39:8: Unused import - java.util.Collections. [UnusedImports]"

@bstansberry bstansberry merged commit 52cae17 into wildfly:main Jul 23, 2023
14 checks passed
@bstansberry
Copy link
Contributor

Thanks @pferraro and @yersan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
3 participants