`EnvironmentVariables` are only visible to the test's main thread

[flemming-n-larsen]

I am trying to use EnvironmentVariables from a Thread. But I have trouble with reading out the values of the set environment variables from a thread, which are null when reading those.

I made a very simplifed test case, that can be used for showing the issue:

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import uk.org.webcompere.systemstubs.environment.EnvironmentVariables;
import uk.org.webcompere.systemstubs.jupiter.SystemStub;
import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension;

@ExtendWith(SystemStubsExtension.class)
class ATest {
    @SystemStub
    private final EnvironmentVariables envVars = new EnvironmentVariables();

    @Test
    void test() {
        envVars.set("FOO_BAR", "foo bar");
        System.out.println("from test(): " + System.getenv("FOO_BAR"));

        new AThread().start();
    }

    @ExtendWith(SystemStubsExtension.class)
    class AThread extends Thread {
        @SystemStub
        private final EnvironmentVariables envVars = new EnvironmentVariables();

        @Override
        public void run() {
            envVars.set("FOO_BAR", "foo bar");
            System.out.println("from AThread.run(): " + System.getenv("FOO_BAR"));
        }
    }
}

When running the test above, these lines will be written out:

from test(): foo bar
from AThread.run(): null

So when I set the env var "FOO_BAR" to "foo bar" and read it out within the test method, test(), the correct value is written out, i.e "foo bar"

But when do the same thing from within a thread (within the run() method), "FOO BAR" is written out as null. What is the trick to set an environment variable and read it out correctly from within a thread?

I could not find any good example in the documentation for how to make this work.

I am using this version of the library:

uk.org.webcompere:system-stubs-jupiter:2.0.1

[ashleyfrieze]

I can’t see why you’re doing this. Your thread has a different lifecycle to the test, and just because you’ve decorated it with the system stubs extension, it doesn’t mean that the extension will run. JUnit does not know about your thread class.

The environment variable object can be turned on and off so it’s stubbing or not stubbing manually. Or you can use the withEnvironment construct, which may better suit your needs. However, as the environment is global, you’ll get odd results across threads, so avoid this sort of thing.

what are you actually trying to achieve with threads?

[flemming-n-larsen]

I know that my threads have another lifecycle than the tests. But in my case, I need to test the behavior of threads, which does read various system environment variables (not in my control).

Hence, I am trying to find a library (like system-stubs) that can help me mocking the environment variables in a way that make these accessible from the threads. The ATest class is just an extremely simplified version of what I am trying to achieve. 🙂

I know that I can use properties instead of environment variables, but I need to read environment variables in my case.

I will have another look at the withEnvironment construct and see if it can help med out. 👍

[ashleyfrieze]

Hi @flemming-n-larsen - if you're going to write a test to test a thread, then the thread should execute and finish before the @Test annotated method has completed. Otherwise, you won't be able to write assertions on the outcome of the thread.

In your first example, the fact you were calling Thread.start at the end of the test meant that the test immediately ended. The plugin for JUnit 5 is set to dispose of the environment variables and return the environment to normal when the test ends. This is why your thread couldn't see the environment variables you'd set.

Had your test used https://www.baeldung.com/java-thread-join then the thread would have had access to the test environment variables.

Within the example thread you wrote, there was no activation of the EnvironmentVariables object. Had you use d execute on it with a lambda containing the code to run within the environment, or called its setup method to turn it on (followed by teardown when you're done), then it would have worked differently.

Having said all of this, I feel like the issue is entirely around test design and very little to do with environment variable stubbing.

You want to test threads but why? How can you control these threads during a test so you can set up before them, test their outcome, and tidy up after them?

[flemming-n-larsen]

Hi @ashleyfrieze

Thank you for taking your time to help my out, and lecture me in writting JUnit tests. Long story short, I know how to make proper unit test in Java.

I admit I wrote the test as short and simple as possible to show the issue I was trying to solve:

• How can I set/change/update environment variables in my unit tests, so I can read them out via System.getenv() from a thread?

Being feed up with clean code, TDD, BDD, etc. I just want something straightforward and simple that works out of the box. Unfortunately, it does not seem to be easy to do with system-stubs, so I replaced it with JUnit Pioneer instead, and voila - problem solved. 🙌

Here is my updated test, which is a bit shorter

import org.junit.jupiter.api.Test;
import org.junitpioneer.jupiter.SetEnvironmentVariable;

@SetEnvironmentVariable(key = "FOO_BAR", value = "foo bar")
class ATest {

    @Test
    void test() {
        System.out.println("from test(): " + System.getenv("FOO_BAR"));

        new AThread().start();
    }

    class AThread extends Thread {
        @Override
        public void run() {
            System.out.println("from AThread.run(): " + System.getenv("FOO_BAR"));
        }
    }
}

which outputs:

from test(): foo bar
from AThread.run(): foo bar

This means a AThread instance is able to read out the FOO_BAR environment variable, and I am able to use this for my real test units with the issue, I was trying to solve.

[ashleyfrieze]

Hey @flemming-n-larsen - JUnit Pioneer is a perfectly good project. I'm not sure if it works with JDK16+ yet. It can only set environment variables to values provided at compile time, where SystemStubs can set environment variables from values provided at runtime too.

Having seen your JUnit Pioneer solution, I'd suggest you could have achieved similar by making your EnvironmentVariables object static rather than instance level.

What you saw as lecturing on TDD/JUnit, was instead an attempt for me to discuss test lifecycle with you.

An instance level SystemStubs object has the lifecycle of an individual test method. A static one has the lifecycle of the whole test fixture.

If you give examples which appear to execute code outside of the lifecycle of the JUnit test, then as someone giving you support, it's my job to:

• point this out
• ask you what your actual use case is

Consider this an attempt to solve the XY problem https://xyproblem.info/

I'll close this question, because I think we're done here.

[ashleyfrieze]

There is a real issue here. I've demonstrated this with a meaningful test in #46 and I can see it relates to a root cause in Mockito https://github.com/mockito/mockito/issues/2142

We'll record this as a bug. If you use an earlier version of SystemStubs - 1.2 (https://search.maven.org/artifact/uk.org.webcompere/system-stubs-parent/1.2.0/po) then this issue won't arise, but you won't be able to use it with JDK 16.

[cmatts-projects]

Hi Ashley,

Will you be fixing this bug?

I hit this in a "real world" scenario. I have a Kinesis client that uses a TimerTask thread to read from the stream and collect messages. Using v1.2.0 I can test that the messages are collected asynchronously fine, but with v2.0.2 the stream reading thread fails because it can't see the env vars.

For now I can get away with using the older version with the "--add-opens java.base/java.util" option for JDK 16, but this bug would stop me from upgrading to newer versions of this library.

Calvin

[ashleyfrieze]

@cmatts-projects - if it were an easy fix, I'd fix it.

There's a certain amount of being snookered by the changes in the module system, and the available ways to hack in the mocks required to control the environment variables.

The issue with the v2 release train is that we're using static mocks from Mockito to modify calls to get environment variables. This is where the thread-specific nature arises. The static mocks appear to be restricted to the current thread.

A possible future change in Mockito might fix this, as might trying to steal the bits of mockito used for static mocking and avoid the thread-specific nature of it. It's a big change and one I've not got the time to get started with. Happy to consider PRs if you fancy it.

You're right that JDK 16 will still allow you to use the v1 release version of this with the exclusions. That's the recommended fix for this issue for now. However, with JDK21 coming out soon, we need to find ways to fix this problem into the future.

Happy to get suggestions on what might be possible.

[cmatts-projects]

No worries.

I think with the direction that the JDK is going we may just need to rethink how we structure some code to make it more testable. We may need to rely on more mocking and less testing with real classes :disappointed:

Calvin

[ashleyfrieze]

@cmatts-projects - I think there's an interesting conundrum here. Mocking environment variables is really a nuclear option. Dependency injection is way easier at test and design time than global dependencies like environment variables.

If we're using Spring, then system properties can also be used in place of environment variables to set things, so long as the environment variables are not set at test time.

I think the future is less mocking, not more...

[reftel]

We´re hitting this issue in tests we have that test a Dropwizard application in its entirety. These tests have a static @ClassRule DropwizardAppRule that starts the application, and a bunch of @Test methods that test various things we can´t really test in isolation (e.g. interaction between filters and resources). The application starts many, many threads, some of which read environment variables, among other things in order to substitute values in its config files. Testing with actual environment variables (instead of, say, generating a config file with hard-coded values on the fly) is a valid test, since this is functionality that the ops team relies on. These tests are working fine for us on Java 11 (with system-stubs 1.x or system-rules), but fail with system-stubs 2.x, which makes it one of the final blockers for us to switch to Java 17.

[ashleyfrieze]

@reftel yeah. This issue sucks. Similarly, DropWizard has a rather nasty way of just using environment variables, rather than using a mechanism more like Spring’s, which allows both environment variables and system properties to be used as placeholders.

I think you have two problems here.

1. You need to poke some values into placeholders
2. You have random threads feeding themselves directly from the environment

I don’t think the second should be happening. Fix that and you’ve got an easier problem. Your configuration ought to be injected once globally from the main thread. How much of this is DropWizard? And how much is your own code?

If you can build a simplified example project that you can share that solves the problem, I can look into it for other options you might take.

Unfortunately, this is only going to get worse as Java hardens.

[reftel]

Thanks for your quick and detailed reply! The issue is not really DropWizard reading environment variables - reading from properties is built in (see https://www.dropwizard.io/en/latest/manual/core.html#config-override ), and implementing and passing a StringSubstitutor that reads its values form somewhere else would be rather trivial. (There are some limitations to the property overrides with regards to arrays, but they haven´t bitten us very hard yet) The thing is, we have requirements on our applications from other parts of the organization that they should use environment variables to configure themselves. For reasons of modularization, we often do this in parts of code that use those variables, whether they run off the main thread or not. Spreading knowledge of what they need to read and from where to other parts of the code base would break encapsulation for little benefit. So, it´s our own code. I realize there are different schools of thought here, but to me, changing production code to fit test code is backwards, which is why we´re using things like system-stubs in test suites that needs them. As for a reduced test case, the one you wrote in PR #46 is pretty much exactly what our code would look like. If Java some day hardens to the point where we can´t override the environment from within, we could always to it from without via a LD_PRELOAD library with a back-channel for the testcases to specify how it should lie to the JVM, but I hope we won´t have to go that far (again... ;-) ).

[ashleyfrieze]

@reftel thanks for the input. I think it's worth looking again at what can be done with Mockito to static mock bits of the environment. It's possible more recent versions of it have fewer limitations.

However, I think the problem you've raised with your production code is an interesting one worth reviewing.

1. The absence of any way to mock environment variables stops you moving to Java 17+
2. You have some code that's trying to stay encapsulated
3. You don't want to hack in testability
4. But your production code spins off multiple threads which independently read environment variables

So your production code, which is coupled to hard-to-mock environment variables, is the reason you can't move to Java 17.

So, let's look at this problem a moment.

Why does your production code NEED environment variables? They're one way of injecting values, but, they're not the only way.

Would your production code be able to use https://github.com/webcompere/lightweight-config/tree/main to inject itself with values? This has the benefit of allowing the placeholders to be resolved from system properties OR environment variables?

Can you have your System.getenv calls use some library which falls back to system properties if environment variables aren't available?

Or are you missing an IoC/DI pattern here?

It's wrong to say that components lose encapsulation if properties can be injected from outside.

I've had some success using Guice dependency injection within DropWizard and perhaps that's a missing pattern from your code?

As it is, the need to be coupled with System.getenv is the reason you're stuck on earlier versions of Java, so alternative patterns are worth considering.

Had you chosen SpringBoot over DropWizard, you wouldn't have this problem, since you can use system properties in place of environment variables to assign things at test time (even though you may prefer not to). I'm not saying switch to SpringBoot, but more that other design patterns don't suffer this problem while still allowing components to be themselves.

[reftel]

Well, the use of environment variables is a requirement from other parts of the organization. We cannot switch to something else on our own. (And so, choosing a different framework would naturally not make the requirement to read environment variables go away). We could add a layer of indirection between the code that reads the environment variables and the environment, but that means adding complexity to the production code because of limitations in one of the test suites (our other test suites, e.g. the Postman collections that we run against deployed instances couldn´t care less about how hard it is to mock the environment in Java), so I´d go quite far to avoid it. Anyway, after thinking a bit more, I think I have work-arounds in the tests for at least most of the cases where we read the environment from whole-application tests (e.g. some of the things tested from there were put there only because it was more convenient than in more isolated unit tests), possibly all of them. I´ll give it a try.

[ashleyfrieze]

@reftel @cmatts-projects @flemming-n-larsen - I'm working on a fix for this. See the attached PR. #46

[ashleyfrieze]

This should be fixed by release 2.1.0 - please re-test and feedback.

[reftel]

Wow, awesome! Thanks for implementing this. I'll try it out as soon as I'm back at the office.

[flemming-n-larsen]

This should be fixed by release 2.1.0 - please re-test and feedback.

I have tried out version 2.1.0 and can confirm that this version works for me (and with my sample code from this thread). It was not working on version 2.0.2 (tested for reference).

Thank you for putting time into fixing this issue. ❤️

I have been using junitpioneer until now, but I ran into problems some weeks ago when upgrading to Java 17, due to the Java reflection restrictions. Love to see that System Stubs does not have this problem! 💪 So I will shift to System Stubs now. 😊

[reftel]

Tried it out on our codebase, and it works with no issues. Thank you so much for your work on this! =)

[ashleyfrieze]

@reftel - I'm delighted!