MP Config TCK - Fix excluded tests

[kenfinnigan]

Currently, the below tests are excluded from execution when running the MP Config TCK. We need to either fix Quarkus so the test passes or challenge and fix the TCK if it's a problem there.

• ArrayConverterTest.*
• ConfigProviderTest.testInjectedConfigSerializable
• ConfigProviderTest.testPropertyConfigSource
• ConfigProviderTest.testEnvironmentConfigSource

[dmlloyd]

microprofile/microprofile-config#446 covers the EmptyValuesTest ones.

[kenfinnigan]

@dmlloyd @jmesnil I was taking a look at ConfigProviderTest.testPropertyConfigSource this afternoon and I'm not sure if the behavior is a problem with Quarkus, the TCK, or even SmallRyeConfig.

Basically the test iterates over system properties and verifies that the values from System.getProperties() are the same as config.getValue() retrieved from the system property config source.

What's weird is that the value for env.MAVEN_CMD_LINE_ARGS contains a leading space in the value when coming from the ConfigSource but doesn't from System.getProperties() in the test class, even though the ConfigSource uses System.getProperty() inside.

Any thoughts?

[dmlloyd]

There's an actual property called env.MAVEN_CMD_LINE_ARGS? If so, we're not setting that AFAIK; Maven must be. And perhaps they're inserting a leading space then?

[kenfinnigan]

I believe it's set by Maven.

I'm just confused how accessing it directly through system properties, and through system properties in the config source, lead to one having a leading space and one not

[dmlloyd]

Is there a log output I could look at without having to build everything?

[kenfinnigan]

Will run the test and capture it for you

[kenfinnigan]

@dmlloyd here's the output from running the single test method: https://gist.github.com/kenfinnigan/ede49a21fd7f144f7d58c65e29b295cc

[dmlloyd]

Hmm that's pretty strange. I think we can rule out SmallRye Config though because that call path would just be System.getProperty.

[kenfinnigan]

Well I guess that's good, but I'm really confused as to how System.getProperty returns different values for that one key on the same system at the same time

[dmlloyd]

I think it's not getProperty.

Here's one hypothesis, which might be a little crazy but it's all I can think of that makes sense: Maven copies all the system and env properties into the Maven property map. They somehow munge the whitespace at that time. Or, at a later point, they set that property by hand for some reason. Then we haplessly copy the build system properties in as a configuration source (we have one for the build system's properties), somehow including that property despite it not being within our namespace.

[kenfinnigan]

So you think we shouldn't be copying any property that doesn't begin QUARKUS?

[dmlloyd]

The intent for that code was in fact to only copy build system properties that started with quarkus.. But I can't think of any other place it could have gone wrong, without attaching a debugger (I'm in the middle of a bunch of code changes so that isn't really a convenient option at this point).

[kenfinnigan]

That's fine, I can dig in to see if I can discover why we're reading more than needed.

Might be a key needs to be passed to SRye Config, maybe

[kenfinnigan]

@dmlloyd so I figured out what's going on with the system properties.

ExpandingConfigSource when it creates the expression compiler, doesn't set a flag for "no trim", which means the value is trimmed. Which leads to our mismatch.

Is it ok to add a no trim flag to the expression creation?

[dmlloyd]

Sure, that should be fine.

[kenfinnigan]

@dmlloyd ConfigProviderTest.testInjectedConfigSerializable fails because Expression isn't serializable.

Does it make sense to make it serializable in SmallRye Common and switch Quarkus to use that instead of WF common? Are the two "identical" enough to do that?

[dmlloyd]

It might be easier to make the ExpandingConfigSource properly serializable by way of writeReplace/readResolve to reconstruct an empty cache. We don't really want to serialize the cache in any event.