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

Test - stale config values #3089

Closed
mkouba opened this issue Jul 4, 2019 · 2 comments
Closed

Test - stale config values #3089

mkouba opened this issue Jul 4, 2019 · 2 comments
Labels
area/testing kind/bug Something isn't working
Milestone

Comments

@mkouba
Copy link
Contributor

mkouba commented Jul 4, 2019

When reviewing #2940 I found out we're still having problems with stale config values during Arquillian tests. I'm not quite sure how to fix this problem but here are my observations:

  1. org.eclipse.microprofile.config.ConfigProvider.getConfig() returns a Config obtained from a ConfigProviderResolver (which is stored in a static field and is initialized once)
  2. ConfigProvider.getConfig() assumes the correct config is identified using the current TCCL
  3. In quarkus we do register a SmallRyeConfig for the current TCCL here: https://github.com/quarkusio/quarkus/blob/master/core/deployment/src/main/java/io/quarkus/deployment/steps/ConfigurationSetup.java#L236
  4. However, during the RUNTIME_INIT we do set a new SimpleConfigurationProviderResolver instance using ConfigProviderResolver.setInstance(ConfigProviderResolver)
  5. The problem is that org.eclipse.microprofile.config.ConfigProvider.INSTANCE points to the old instance and that's why we introduced a hack where the current config is stored in a static field: SimpleConfigurationProviderResolver.config
  6. This hack was obviously not enough because we introduced QuarkusConfigProducer which is using
    ConfigProviderResolver.instance().getConfig() instead of ConfigProvider.getConfig(): https://github.com/quarkusio/quarkus/blob/master/extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/QuarkusConfigProducer.java#L29
  7. The current issue is that if there is a STATIC_INIT build step that attempts to obtain a value of a config property and the property had a different value in the previous test (using the same JVM instance) the latter test will see a stale value during the augmentation phase.
    It's a bit of a mess and I think that we should really clean this up.

And @dmlloyd's comment:

my current thoughts, in no particular order, are:

  • our SimpleConfigProviderResolver should be set as early as possible, so that ConfigProvider.getConfig always returns a consistent answer compared to ConfigProviderResolver.instance()
  • we should never change it after that!
  • when the config object needs to be changed, we should update SimpleConfigurationProviderResolver via the methods on that class
  • because static init is so weird compared to the rest of run time, I still think that we should have no global configuration object registered during that phase; instead prefer some static getBuildTimeConfig() method which gets the build time config

at least, that would avoid some confusion
I guess that would solve the underlying problem, and probably introduce a few superficial problems

See the original conversation: https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/Test.20config.20issue/near/169396567

@gsmet
Copy link
Member

gsmet commented Nov 13, 2019

@dmlloyd I added this one to your config project.

@dmlloyd
Copy link
Member

dmlloyd commented Nov 13, 2019

Thanks, I've incorporated it in to the Big PR.

@gsmet gsmet added this to the 1.1.0 milestone Nov 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants