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

CHE_INFRASTRUCTURE and CHE_MULTIUSER vars can be now passed as system #10801

Merged
merged 1 commit into from Aug 16, 2018

Conversation

rhopp
Copy link
Contributor

@rhopp rhopp commented Aug 16, 2018

What does this PR do?

Before this PR, properties CHE_MULTIUSER and CHE_INFRASTRUCTURE had to be set as environmental variables. After this PR is merged, it will be possible to set them as system variables.

In other words - before this PR, it was not possible to start tests with -Dche.infrastructure=openshift for example... After this PR this should be possible.

From my testing, this PR should not break any flow (a.k.a. setting env variable CHE_INFRASTRUCTURE will be enough)

What issues does this PR fix or reference?

Release Notes

Docs PR

@rhopp rhopp requested a review from dmytro-ndp as a code owner August 16, 2018 09:13
@riuvshin
Copy link
Contributor

Can one of the admins verify this patch?

1 similar comment
@riuvshin
Copy link
Contributor

Can one of the admins verify this patch?

properties.

Signed-off-by: Radim Hopp <rhopp@redhat.com>
@@ -128,12 +128,6 @@ public boolean test(Map.Entry<K, V> entry) {
name = name.replace('_', '.');
name = name.replace("=", "_");

// convert value of CHE_INFRASTRUCTURE to upper case to comply with Infrastructure
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As now, the che.infrastructure can be defined also as system property, I had to move this logic somewhere else (InMemoryTestConfiguration). If the new place is not the best one, please let me know.

@dmytro-ndp
Copy link
Contributor

Looks good for me. Lets run CI tests.

Copy link
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it was impossible to run selenium tests on CI because of problem with running Eclipse Che on OCP, but locally they work as expected.

@dmytro-ndp dmytro-ndp merged commit feb0065 into eclipse-che:master Aug 16, 2018
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Aug 16, 2018
@benoitf benoitf added this to the 6.10.0 milestone Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants