Fix php tag fold test (attempt 2)

[mbien]

The race condition strikes back!

This particular php tests requires too many restarts, I can't stabilize master at all atm:

netbeans/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/FoldingTest.java

Line 88 in 0c8313e

checkFolds("testfiles/parser/foldingUses.php");

So I took another look at #5658, and I believe all I did back then was to change the timing without fixing the underlying issue. My mistake was to think that getSource().getDocument(true) would hold the document internally for the next getDocument(false) call - but this is not the case. So all the last "fix" did was to read a document and forget about it again, the timing however helped the tests to pass.

This is also annoying to debug since this usually works when the debugger is attached.

So I think there are two obvious ways how to fix this:

• actually cache the document in Source.getDocument(true) for subsequent Source.getDocument(false) calls, and eager load the document in the test like attempted in 5658
• or call Source.getDocument(true) in FoldingScanner (might potentially have an performance impact)

PR implements currently the second option

[junichi11]

Looks good to me. Thank you for your work!

[mbien]

@junichi11 would be good to observe this to check if it causes any performance issues.

It could be worth checking how often source.getDocument(false) would be null here during regular project development. If its very often the case, we could try to solve this differently (e.g bullet point 1 of the PR)

[junichi11]

@mbien
Or, change to true only when we test?

diff --git a/php/php.editor/src/org/netbeans/modules/php/editor/csl/FoldingScanner.java b/php/php.editor/src/org/netbeans/modules/php/editor/csl/FoldingScanner.java
index fb565eb64a..730455feb3 100644
--- a/php/php.editor/src/org/netbeans/modules/php/editor/csl/FoldingScanner.java
+++ b/php/php.editor/src/org/netbeans/modules/php/editor/csl/FoldingScanner.java
@@ -141,6 +141,7 @@ public final class FoldingScanner {
 
     private static final String LAST_CORRECT_FOLDING_PROPERTY = "LAST_CORRECT_FOLDING_PROPERY"; //NOI18N
     private static final boolean FOLD_PHPTAG = !Boolean.getBoolean("nb.php.editor.doNotFoldPhptag"); // NOI18N NETBEANS-5480
+    static volatile boolean IS_UNIT_TEST = false;
 
     public static FoldingScanner create() {
         return new FoldingScanner();
@@ -179,7 +180,8 @@ public final class FoldingScanner {
             program.accept(new FoldingVisitor(folds));
             Source source = phpParseResult.getSnapshot().getSource();
             assert source != null : "source was null";
-            Document doc = source.getDocument(true);
+            // avoid getting `null` in unit testing
+            Document doc = source.getDocument(IS_UNIT_TEST);
             if (FOLD_PHPTAG) {
                 processPHPTags(folds, doc);
             }
diff --git a/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/FoldingTest.java b/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/FoldingTest.java
index 083d201589..b7ad8a515c 100644
--- a/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/FoldingTest.java
+++ b/php/php.editor/test/unit/src/org/netbeans/modules/php/editor/csl/FoldingTest.java
@@ -30,6 +30,12 @@ public class FoldingTest extends PHPTestBase {
         super(testName);
     }
 
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        FoldingScanner.IS_UNIT_TEST = true;
+    }
+
     public void testFoldingMethod() throws Exception {
         checkFolds("testfiles/parser/foldingMethod.php");
     }

[mbien]

yes but making the program aware of the test is always a bit ugly. The program should, if possible, never know that it is tested so that the results are the same as in real world. This is also a valid option but I would try to use it only if we really have to.

[junichi11]

OK. Perhaps, someone would report performance issues if there are them :) Thanks!

[mbien]

but something weird is happening here, we might have to revert this:

both micronaut and LSP jobs are failing after this merge and I can't get them green again. I can reproduce it locally, if i set back to false, MicronautGradleArtifactsImplTest is green, true makes it red...

[junichi11]

Umm, I set back to false but unit tests failed...

Product Version: Apache NetBeans IDE 19
Java: 19.0.2; OpenJDK 64-Bit Server VM 19.0.2+7-Ubuntu-0ubuntu322.04
Runtime: OpenJDK Runtime Environment 19.0.2+7-Ubuntu-0ubuntu322.04
System: Linux version 6.2.0-36-generic running on amd64; UTF-8;

[mbien]

yes, there is something else going on, I try to debug it here: mbien#1

[mbien]

(this has likely nothing to do with this PR or any recent PRs, both micronaut and LSP tests which fail use gradle which is suspicious, the runner images, jdk version, everything remained unchanged since the last successful build on master)

[mbien]

#6673

[junichi11]

Great. Thanks!

[mbien]

thanks for review - merging for now to check if it helps.