dawidwys commented on a change in pull request #15279:
URL: https://github.com/apache/flink/pull/15279#discussion_r598613994
##########
File path:
flink-core/src/test/java/org/apache/flink/testutils/ThrowOnDoubleMemoryFreeing.java
##########
@@ -19,32 +19,28 @@
package org.apache.flink.testutils;
import org.apache.flink.core.memory.MemorySegment;
-import org.apache.flink.core.testutils.CommonTestUtils;
import org.apache.flink.util.ExternalResource;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.Properties;
/**
* A rule that can be used to enable throwing exception on multiple freeing of
{@link
* MemorySegment}.
*/
public class ThrowOnDoubleMemoryFreeing implements ExternalResource {
- private Map<String, String> systemEnv = new HashMap<>();
+ private Properties systemProperties;
@Override
public void before() throws Exception {
- systemEnv = System.getenv();
- CommonTestUtils.setEnv(
-
Collections.singletonMap(MemorySegment.CHECK_MULTIPLE_FREE_PROPERTY, ""),
false);
+ systemProperties = System.getProperties();
+ System.setProperty(MemorySegment.CHECK_MULTIPLE_FREE_PROPERTY, "");
Review comment:
That's a very good point! The current solution might break other tests :(
I would not add the `Assume` as we do not need to ignore the tests. The
tests will still pass even without the throwing the exception. The problem is
that without the exception they would pass even without the fix in place, but I
guess we can live with it for now.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
