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-4369] Fixing JDK 13 test issue #3702

Merged
merged 1 commit into from Apr 8, 2019
Merged

Conversation

@ropalka
Copy link
Contributor

ropalka commented Mar 12, 2019

@ropalka

This comment has been minimized.

Copy link
Contributor Author

ropalka commented Mar 12, 2019

Please review @darranl

@wildfly-ci

This comment has been minimized.

Copy link

wildfly-ci commented Mar 12, 2019

Core - Full Integration Build 8408 outcome was FAILURE using a merge of 95e2cfe
Summary: Exit code 1 (Step: Build & test core (Maven)) (new) Build time: 00:01:13

@darranl

This comment has been minimized.

Copy link
Contributor

darranl commented Mar 12, 2019

This seems to be suffering from a lot of CI problems.

….Provider was replaced with sun.security.ssl.SunJSSE.
@ropalka ropalka force-pushed the ropalka:WFCORE-4369 branch from 95e2cfe to a52595f Mar 13, 2019
@ropalka

This comment has been minimized.

Copy link
Contributor Author

ropalka commented Mar 14, 2019

All should be OK now @darranl

@bstansberry bstansberry requested a review from darranl Mar 25, 2019
@darranl

This comment has been minimized.

Copy link
Contributor

darranl commented Mar 25, 2019

@mchoma Do you have an IBM environment available to double check this one? I would like to get it merged but our CI doesn't cover the IBM JDK.

@bstansberry

This comment has been minimized.

Copy link
Contributor

bstansberry commented Mar 26, 2019

@mchoma

This comment has been minimized.

Copy link
Contributor

mchoma commented Mar 26, 2019

we dont have ibm jdk11 neither.

<!-- for needs of SaslTestCase and KeyStoresTestCase -->
<subsystem xmlns="urn:wildfly:elytron:7.0" default-ssl-context="ClientSslContextNoAuth">
<providers>
<provider-loader name="ManagerProviderLoader" class-names="sun.security.ssl.SunJSSE"/>

This comment has been minimized.

Copy link
@mchoma

mchoma Mar 26, 2019

Contributor

so that is only one change compared to tls-sun.xml. Could be tls-sun.xml parametrized?

This comment has been minimized.

Copy link
@ropalka

ropalka Mar 26, 2019

Author Contributor

I'm not that familiar with WildFly configuration mock test support @mchoma .
@darranl @bstansberry @kabir is it possible to parametrize subsystem test configurations
based on condition comparing 'java.specification.version' JVM property?

This comment has been minimized.

Copy link
@bstansberry

bstansberry Mar 26, 2019

Contributor

If the attribute supports expressions you can put an expression in the xml and then set a system property before running the test; e.g. the code you have now that's selecting which config file could instead set a property.

I haven't looked, but these may not support expressions. Historically we didn't for 'class name' attributes. For no great reason really. This is the first good use case for using them in that kind of attribute that I've seen.

This comment has been minimized.

Copy link
@ropalka

ropalka Mar 27, 2019

Author Contributor

@mchoma I'm not going to investigate this. Let's leave the proposed changes as they are - it's all just about tests and not implementation.

This comment has been minimized.

Copy link
@mchoma

mchoma Mar 27, 2019

Contributor

Ok I dont insist. Setting /elytron/provider-loader/class-names/expressions-allowed = true would help. But as Brian said it is everywhere else false. So we should be consistent.

This comment has been minimized.

Copy link
@bstansberry

bstansberry Mar 27, 2019

Contributor

I don't mind changing an expressions-allowed setting if that's what Darran/Farah/Jeff Mesnil prefer. There's really not a good reason for having those as false and if there's a reasonable, understandable use case for true for a particular attribute I don't think we have to enforce a rule against it.

I agree a change like that is beyond the scope of this PR though.

This comment has been minimized.

Copy link
@mchoma

mchoma Mar 27, 2019

Contributor

Thinking again. Shouldn't it work without provider-loader name="ManagerProviderLoader" by default?. So by default using just system providers? Or is specifying provider loader what is tested here?

This comment has been minimized.

Copy link
@darranl

darranl Mar 27, 2019

Contributor

Off topic for this PR but when we started defining attributes with expression support enabled and disabled it was primarily to avoid references to other parts of the model being handled as an expression, but at the time we didn't have support for capabilities.

I wonder if we are getting nearer to a point where expression support is enabled unless the attribute is a capability reference.

@mchoma
mchoma approved these changes Mar 27, 2019
@bstansberry

This comment has been minimized.

Copy link
Contributor

bstansberry commented Mar 27, 2019

I've commented on this PR as part of general discussing but am not approving or disapproving, so don't wait for me. Looks like @darranl is reviewing.

The custom IBM JDK 8 test job I ran against this branch (link above) produced the same results as the normal nightly runs. We get 13 failures, which would be good to sort, but that's out of scope for this PR. No failures in the elytron subsystem tests.

@darranl
darranl approved these changes Apr 5, 2019
@darranl darranl merged commit a988279 into wildfly:master Apr 8, 2019
7 checks passed
7 checks passed
Full integration - Linux Finished TeamCity Build WildFly Core / Pull Request / WildFly Core Full - Integration Linux - JDK 8 : Tests passed: 4825, ignored: 133
Details
Full integration - Windows Finished TeamCity Build WildFly Core / Pull Request / WildFly Core Full - Integration - Windows - JDK 8 : Tests passed: 4818, ignored: 138
Details
Linux - JDK 11 (Pull Request) - merge TeamCity build finished
Details
Linux - JDK 8 (Pull Request) - merge TeamCity build finished
Details
Linux - Security Manager - JDK 8 (Pull Request) - merge TeamCity build finished
Details
Windows - JDK 11 (Pull Request) - merge TeamCity build finished
Details
Windows - JDK 8 (Pull Request) - merge TeamCity build finished
Details
@ropalka ropalka deleted the ropalka:WFCORE-4369 branch Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.