4.x: Enable microprofile/tests/tck/tck-config/src/test/tck-suite.xml tests #8173
Conversation
oracle-contributor-agreement
bot
added
the
OCA Verified
jbescos
force-pushed
the
issue6105
branch
2 times, most recently
from
146f4fb
to
82c58c7
Compare
…tests helidon-io#8173 Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
| } catch (NoSuchElementException e) { | ||
| // Property expression does not resolve | ||
| return Optional.empty(); | ||
| return Optional.of(new ConfigValueImpl(propName, null, rawValue, name, ordinal)); |
There was a problem hiding this comment.
This is the point 1 I was referring before.
| @@ -313,35 +334,47 @@ private <T> T convert(String propertyName, Class<T> type, String value) { | |||
| } | |||
| } | |||
|
|
|||
| private Optional<ConfigValue> findConfigValue(String propertyName) { | |||
| private Optional<ConfigValue> findConfigValue(String propertyName, Optional<String> profiledPropertyName) { | |||
There was a problem hiding this comment.
we do not use optional parameters. Also it does not make sense in this case, as you wrap it just to unwrap it later. Please use null here, as it is a private method (null is not allowed in public API only).
| String rawValue = value; | ||
| String name = source.getName(); | ||
| int ordinal = source.getOrdinal(); | ||
| final String propName = selectedProperty; |
There was a problem hiding this comment.
This name is not descriptive. Use selectedPropertyFinal or something similar, with a comment "required final variable, as it is used in lambdas"
| @@ -126,70 +129,88 @@ public <T> T getValue(String propertyName, Class<T> propertyType) { | |||
| @SuppressWarnings("unchecked") | |||
| @Override | |||
| public <T> Optional<T> getOptionalValue(String propertyName, Class<T> propertyType) { | |||
| String profiledPropertyName = null; | |||
There was a problem hiding this comment.
This statement does not make sense here - the next if is checking for profile being null. So just move the content of the if statement after the if checking if profile is null and remove the check.
| + " in source " + source.getName() | ||
| + " and it is empty (removed)"); | ||
| } | ||
| return Optional.empty(); | ||
| return Optional.of(new ConfigValueImpl(propName, null, rawValue, name, ordinal)); |
There was a problem hiding this comment.
Why is this not returning an empty optional? I am missing an explanation of this change (and once more further down).
There was a problem hiding this comment.
In this particular case you are right, it can return empty Optional, so I have reverted it.
For the other case:
[ERROR] Failures:
[ERROR] CDIPropertyExpressionsTest>Arquillian.run:138->badExpansion:84 expected [test] but found [null]
[ERROR] PropertyExpressionsTest>Arquillian.run:138->noExpressionButConfigValue:145 expected [test] but found [null]
[ERROR] PropertyExpressionsTest>Arquillian.run:138->noExpressionComposedButConfigValue:171 expected [test] but found [null]
I am guided by the test results and then stepping back from there to see what is causing the failure. The tests do not give any explication, so I am afraid I cannot tell you why it has to behave in this way.
jbescos
force-pushed
the
issue6105
branch
2 times, most recently
from
0ebf6f6
to
e84e8cd
Compare
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
jbescos
force-pushed
the
issue6105
branch
2 times, most recently
from
7a17219
to
6dc63dd
Compare
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
|
@tomas-langer kindly reminder to review this. |
#6105
Description
We were waiting for microprofile-config 3.1 to enable one test. This version made other issues that are fixed in this PR too.
There were 2 types of issues:
Documentation
N/A