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

Add Annotation for System Environment Variables #167

Closed
Hancho2009 opened this issue Jan 23, 2020 · 4 comments · Fixed by #174
Closed

Add Annotation for System Environment Variables #167

Hancho2009 opened this issue Jan 23, 2020 · 4 comments · Fixed by #174
Assignees
Labels
⚙️ component: Jupiter Issues coming from (internal/official) Jupiter features etc. 🚦 status: in progress 🏗️ type: enhancement

Comments

@Hancho2009
Copy link
Contributor

Hancho2009 commented Jan 23, 2020

Similiar to @SetSystemProperty and @ClearSystemProperty it would be useful if there were a option to set and clear System Enviroment Variables (java.lang.System.getenv).

"We can update [System] Properties at runtime while Environment Variables are an immutable copy of the Operating System's variables." [1].

I found a JUnit 4 Rule that does it [2] and a not released Lambda solution [3] from the same author. Both solutions have this warning "This rule uses reflection for modifying internals of the environment variables map. It fails if your SecurityManager forbids such modifications."

I am not sure about the license stuff for reusing parts of [2] or [3] in this project (see also [4]). Would that be a problem?

[1] https://www.baeldung.com/java-system-get-property-vs-system-getenv
[2] https://github.com/stefanbirkner/system-rules/blob/master/src/main/java/org/junit/contrib/java/lang/system/EnvironmentVariables.java
[3] https://github.com/stefanbirkner/system-lambda/blob/master/src/main/java/com/github/stefanbirkner/systemlambda/SystemLambda.java (WithEnvironmentVariables)
[4] stefanbirkner/system-rules#55 (comment)

@Hancho2009 Hancho2009 changed the title Add Annotation for System Enviroment Variables Add Annotation for System Environment Variables Feb 5, 2020
@Hancho2009
Copy link
Contributor Author

@nipafx
Copy link
Member

nipafx commented Feb 11, 2020

I totally understand why this feature could be useful. At the same time it's always questionable to use reflection to break into internal APIs. And then there's the module system's encapsulation on Java 9+ to deal with.

I think if faced with this decision, it really comes down to four choices:

  1. not write the test at all
  2. fiddle with VMs/containers to set the correct env variables
  3. write your own ad-hoc hack
  4. use Pioneer's hack (if we implement it)

Number 1. would be too bad, not every project can do 2. and 3. surely isn't better than 4. So I'm ok with seeing this get implemented within Pioneer. We just have to make sure that users know that they're relying on a fragile solution. I'm thinking:

  • put a warning into the .adoc
  • put a warning into the Javadoc
  • print a warning once per test suite/class/method (?) execution if the extension is active

Beyond the warning itself it may be helpful to gather alternatives and explain them in the .adoc and mention them everywhere else.

@nipafx nipafx added 🏗️ type: enhancement ⚙️ component: Jupiter Issues coming from (internal/official) Jupiter features etc. 📢 up for grabs labels Feb 11, 2020
@nipafx
Copy link
Member

nipafx commented Feb 11, 2020

@Hancho2009, are you interested to give this a shot? It would mostly be a copy of the other two extensions you mention but with the reflection-based code to actually update the environment variables?

If anybody else wants to work on this, make sure to let us know here.

@Hancho2009
Copy link
Contributor Author

Here is my shot. It works for me with Windows JDK8. It would be good if others with different systems run the tests too, because I have no linux, mac, android etc.

  • print a warning once per test suite/class/method (?) execution if the extension is active

@nicolaiparlog I did not implement this because I did not know exactly how and where. Maybe you can give me an example of the how and a hint for the where.

I have a ProcessEnvironmentClass (Windows) and a SystemEnvClass (Linux (not tested)) way to modify the variables. If the first throw a NoSuchFieldException or ClassNotFoundException I try the second way. If both fail I throw a RuntimeException but do not attach a cause because I cannot attach 2 causes as far as I know.

nipafx pushed a commit that referenced this issue Feb 18, 2020
Add new annotations ClearEnvironmentVariable and SetEnvironmentVariable.
nipafx pushed a commit that referenced this issue Feb 18, 2020
put a warning into the .adoc
put a warning into the Javadoc
nipafx pushed a commit that referenced this issue Feb 18, 2020
replace spaces with tab
nipafx pushed a commit that referenced this issue Apr 7, 2020
The `@ClearEnvironmentVariable` and `@SetEnvironmentVariable`
annotations can be used to clear, respectively, set the values
of environment variables for a test execution. Because the JVM
stores environment variables as an immutable copy of the
operating system's variables, we have to sue reflection to
change them. Appropriate warnings are added to the documentation
and written to the log.

PR: #174
Closes: #167
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚙️ component: Jupiter Issues coming from (internal/official) Jupiter features etc. 🚦 status: in progress 🏗️ type: enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants