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

[WFCORE-6959] test validating final endpoint name with different jboss.server.name and jboss.node.name configurations #6202

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

simkam
Copy link
Contributor

@simkam simkam commented Sep 26, 2024

https://issues.redhat.com/browse/WFCORE-6959

I tried to run this test case with wildfly-core 25.0.2 and it failed due to WFCORE-6956 as expected.

I find interesting that jboss.server.name configured in server configuration (<server xmlns="urn:jboss:domain:20.0" name="custom-server-name">) has precedence over configuration via -Djboss.server.name system property.


package org.jboss.as.test.manualmode.management;

import static org.jboss.as.test.manualmode.expressions.module.TestSecureExpressionsExtension.PARAM_EXPRESSION;
Copy link
Collaborator

@yersan yersan Sep 27, 2024

Choose a reason for hiding this comment

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

I would say it is better to keep tests independent; TestSecureExpressionsExtension is unrelated to what we are doing here, so I would not use a constant defined in an external test, instead it would be better to define it in this class.

@simkam Do you agree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@yersan
Copy link
Collaborator

yersan commented Sep 27, 2024

I find interesting that jboss.server.name configured in server configuration () has precedence over configuration via -Djboss.server.name system property.

I see what you mean, and I agree that the natural behavior would be the system property to override the model value. It has been working as it this since the beginning, so I'm not sure if it makes sense to change this behavior now, especially since we haven't heard any complaints (I guess the domain mode host controller name has the same issue and it cannot be overwritten via a System Property). However, I agree that it is not a natural behavior. CC @bstansberry , maybe he knows a reason for that, also it would be great to heard your opinion about this.

@yersan yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Oct 2, 2024
@yersan yersan merged commit bec2845 into wildfly:main Oct 2, 2024
11 of 12 checks passed
@yersan
Copy link
Collaborator

yersan commented Oct 2, 2024

@simkam thanks!

@simkam simkam deleted the server-name-node-name-test branch October 2, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants