Re: [PR] Enable SurrogatePairLengthTest without forked JVM [xerces-j]

elharo commented on code in PR #116:
URL: https://github.com/apache/xerces-j/pull/116#discussion_r3610562308


##########
tests/schema/config/SurrogatePairLengthTest.java:
##########
@@ -27,12 +28,33 @@
  */
 public class SurrogatePairLengthTest extends BaseTest {
 
-    // Can only test when the property is set
+    private static final String PROPERTY = "org.apache.xerces.impl.dv.xs.useCodePointCountForStringLength";

Review Comment:
   Inline these two string fields



##########
tests/schema/config/SurrogatePairLengthTest.java:
##########
@@ -27,12 +28,33 @@
  */
 public class SurrogatePairLengthTest extends BaseTest {
 
-    // Can only test when the property is set
+    private static final String PROPERTY = "org.apache.xerces.impl.dv.xs.useCodePointCountForStringLength";
+    private static final String LENGTH_ERROR = "cvc-length-valid";
+    private static final Field codePointCountField;
+    
     static {
-        System.setProperty("org.apache.xerces.impl.dv.xs.useCodePointCountForStringLength", "true");
+        try {
+            Field f = TypeValidator.class.getDeclaredField("USE_CODE_POINT_COUNT_FOR_STRING_LENGTH");
+            f.setAccessible(true);
+            codePointCountField = f;
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
     }
     
-    private static final String LENGTH_ERROR = "cvc-length-valid";
+    // Tests run sequentially within a shared JVM, so setUp/tearDown
+    // can safely set and reset the flag before each test.
+    protected void setUp() throws Exception {
+        super.setUp();
+        System.setProperty(PROPERTY, "true");
+        codePointCountField.set(null, true);
+    }
+    
+    protected void tearDown() throws Exception {
+        System.clearProperty(PROPERTY);
+        codePointCountField.set(null, false);

Review Comment:
   do we know false was the original value here?



##########
build.xml:
##########
@@ -835,25 +835,27 @@ Authors:
              <include name="schema/config/IgnoreXSIType_A_C_Test.class"/>
              <include name="schema/config/IgnoreXSIType_C_A_Test.class"/>
              <include name="schema/config/RootSimpleTypeDefinitionTest.class"/>
-             <include name="schema/config/RootTypeDefinitionTest.class"/>
-             <include name="schema/config/UseGrammarPoolOnly_False_Test.class"/>
-          <!-- These tests are failing. Fix them.
-             <include name="schema/config/IgnoreXSIType_C_AC_Test.class"/>
-             <include name="schema/config/IgnoreXSIType_C_CA_Test.class"/>
-             <include name="schema/config/IgnoreXSIType_C_C_Test.class"/>
-             <include name="schema/config/SurrogatePairLengthTest.class"/>
-             <include name="schema/config/UseGrammarPoolOnly_True_Test.class"/>
-             <include name="schema/config/UnparsedEntityCheckingTest.class"/>
-            -->             
+              <include name="schema/config/RootTypeDefinitionTest.class"/>
+              <include name="schema/config/UseGrammarPoolOnly_False_Test.class"/>
+           <!-- These tests are failing. Fix them.
+              <include name="schema/config/IgnoreXSIType_C_AC_Test.class"/>
+              <include name="schema/config/IgnoreXSIType_C_CA_Test.class"/>
+              <include name="schema/config/IgnoreXSIType_C_C_Test.class"/>
+              <include name="schema/config/UseGrammarPoolOnly_True_Test.class"/>
+              <include name="schema/config/UnparsedEntityCheckingTest.class"/>
+             -->
+             <!-- SurrogatePairLengthTest sets a system property in setUp() that

Review Comment:
   This needs to indicate that this test is not safe for parallel execution. 



##########
tests/schema/config/SurrogatePairLengthTest.java:
##########
@@ -27,12 +28,33 @@
  */
 public class SurrogatePairLengthTest extends BaseTest {
 
-    // Can only test when the property is set
+    private static final String PROPERTY = "org.apache.xerces.impl.dv.xs.useCodePointCountForStringLength";
+    private static final String LENGTH_ERROR = "cvc-length-valid";
+    private static final Field codePointCountField;
+    
     static {
-        System.setProperty("org.apache.xerces.impl.dv.xs.useCodePointCountForStringLength", "true");
+        try {
+            Field f = TypeValidator.class.getDeclaredField("USE_CODE_POINT_COUNT_FOR_STRING_LENGTH");

Review Comment:
   why is this all in a static block instead of setUp?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]