You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/07 07:11:50 UTC
[GitHub] [pulsar] eolivelli opened a new pull request, #15957: [fix][tests] TieredStorageConfigurationTests - clear system properties
eolivelli opened a new pull request, #15957:
URL: https://github.com/apache/pulsar/pull/15957
Motivation:
This PR https://github.com/apache/pulsar/pull/15710 introduced a new test case that set some system properties without clearing them.
Modification:
clean up the env, in order to not pollute the execution of the other tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #15957: [fix][tests] TieredStorageConfigurationTests - clear system properties
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #15957:
URL: https://github.com/apache/pulsar/pull/15957#discussion_r890860383
##########
tiered-storage/jcloud/src/test/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/TieredStorageConfigurationTests.java:
##########
@@ -217,9 +217,10 @@ public void overridePropertiesTest() {
System.setProperty("jclouds.region", "jclouds-region");
TieredStorageConfiguration config = new TieredStorageConfiguration(map);
Properties properties = config.getOverrides();
- System.out.println(properties.toString());
assertEquals(properties.get("jclouds.region"), "jclouds-region");
assertEquals(config.getServiceEndpoint(), "http://localhost");
assertEquals(properties.get("jclouds.SystemPropertyA"), "A");
+ System.clearProperty("jclouds.SystemPropertyA");
Review Comment:
better to do that in a finally block
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a diff in pull request #15957: [fix][tests] TieredStorageConfigurationTests - clear system properties
Posted by GitBox <gi...@apache.org>.
Jason918 commented on code in PR #15957:
URL: https://github.com/apache/pulsar/pull/15957#discussion_r894974820
##########
tiered-storage/jcloud/src/test/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/TieredStorageConfigurationTests.java:
##########
@@ -217,9 +217,10 @@ public void overridePropertiesTest() {
System.setProperty("jclouds.region", "jclouds-region");
TieredStorageConfiguration config = new TieredStorageConfiguration(map);
Properties properties = config.getOverrides();
- System.out.println(properties.toString());
assertEquals(properties.get("jclouds.region"), "jclouds-region");
assertEquals(config.getServiceEndpoint(), "http://localhost");
assertEquals(properties.get("jclouds.SystemPropertyA"), "A");
+ System.clearProperty("jclouds.SystemPropertyA");
+ System.clearProperty("jclouds.region");
Review Comment:
Is it better to store the previous value and reset to original state here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a diff in pull request #15957: [fix][tests] TieredStorageConfigurationTests - clear system properties
Posted by GitBox <gi...@apache.org>.
eolivelli commented on code in PR #15957:
URL: https://github.com/apache/pulsar/pull/15957#discussion_r901677944
##########
tiered-storage/jcloud/src/test/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/TieredStorageConfigurationTests.java:
##########
@@ -217,9 +217,10 @@ public void overridePropertiesTest() {
System.setProperty("jclouds.region", "jclouds-region");
TieredStorageConfiguration config = new TieredStorageConfiguration(map);
Properties properties = config.getOverrides();
- System.out.println(properties.toString());
assertEquals(properties.get("jclouds.region"), "jclouds-region");
assertEquals(config.getServiceEndpoint(), "http://localhost");
assertEquals(properties.get("jclouds.SystemPropertyA"), "A");
+ System.clearProperty("jclouds.SystemPropertyA");
Review Comment:
makes sense
##########
tiered-storage/jcloud/src/test/java/org/apache/bookkeeper/mledger/offload/jcloud/provider/TieredStorageConfigurationTests.java:
##########
@@ -217,9 +217,10 @@ public void overridePropertiesTest() {
System.setProperty("jclouds.region", "jclouds-region");
TieredStorageConfiguration config = new TieredStorageConfiguration(map);
Properties properties = config.getOverrides();
- System.out.println(properties.toString());
assertEquals(properties.get("jclouds.region"), "jclouds-region");
assertEquals(config.getServiceEndpoint(), "http://localhost");
assertEquals(properties.get("jclouds.SystemPropertyA"), "A");
+ System.clearProperty("jclouds.SystemPropertyA");
+ System.clearProperty("jclouds.region");
Review Comment:
it theory there should be no "previous" value, because this is a system property
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli merged pull request #15957: [fix][tests] TieredStorageConfigurationTests - clear system properties
Posted by GitBox <gi...@apache.org>.
eolivelli merged PR #15957:
URL: https://github.com/apache/pulsar/pull/15957
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #15957: [fix][tests] TieredStorageConfigurationTests - clear system properties
Posted by GitBox <gi...@apache.org>.
eolivelli commented on PR #15957:
URL: https://github.com/apache/pulsar/pull/15957#issuecomment-1161327169
/pulsarbot rerun-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org