Run Dependant Tests Issue(s)

[Tony White]

The org I am in has a large number of classes (68% of Apex limit used), so the ‘Finding Dependents’ takes a long time (currently at +20mins, I gave up waiting and cancelled it)

Wondering why you did not use the Metadata Dependency API for this - https://developer.salesforce.com/docs/atlas.en-us.api_tooling.meta/api_tooling/tooling_api_objects_metadatacomponentdependency.htm

‌

Error 1 noticed in Logs, this appears numerous times:

2024-07-31 12:57:12,719 [85158477] SEVERE - #c.i.u.c.ThreadingAssertions - WebStorm 2024.1.5 Build #WS-241.18034.50
2024-07-31 12:57:12,719 [85158477] SEVERE - #c.i.u.c.ThreadingAssertions - JDK: 17.0.11; VM: OpenJDK 64-Bit Server VM; Vendor: JetBrains s.r.o.
2024-07-31 12:57:12,719 [85158477] SEVERE - #c.i.u.c.ThreadingAssertions - OS: Mac OS X
2024-07-31 12:57:12,719 [85158477] SEVERE - #c.i.u.c.ThreadingAssertions - Plugin to blame: Illuminated Cloud 2 version: 2.3.2.2
2024-07-31 12:57:12,729 [85158487] SEVERE - #c.i.u.c.ThreadingAssertions - Read access is allowed from inside read-action only (see Application.runReadAction()); see https://jb.gg/ij-platform-threading for details
Current thread: Thread[ApplicationImpl pooled thread 652,4,main] 1187717128 (EventQueue.isDispatchThread()=false)
SystemEventQueueThread: Thread[AWT-EventQueue-0,6,main] 1594025498
com.intellij.openapi.diagnostic.RuntimeExceptionWithAttachments: Read access is allowed from inside read-action only (see Application.runReadAction()); see https://jb.gg/ij-platform-threading for details
Current thread: Thread[ApplicationImpl pooled thread 652,4,main] 1187717128 (EventQueue.isDispatchThread()=false)
SystemEventQueueThread: Thread[AWT-EventQueue-0,6,main] 1594025498
at com.intellij.util.concurrency.ThreadingAssertions.createThreadAccessException(ThreadingAssertions.java:180)
at com.intellij.util.concurrency.ThreadingAssertions.softAssertReadAccess(ThreadingAssertions.java:131)
at com.intellij.openapi.application.impl.ApplicationImpl.assertReadAccessAllowed(ApplicationImpl.java:908)
at com.intellij.psi.impl.file.impl.FileManagerImpl.getCachedPsiFile(FileManagerImpl.java:382)
at com.intellij.psi.stubs.StubTreeLoaderImpl.checkLengthMatch(StubTreeLoaderImpl.java:148)
at com.intellij.psi.stubs.StubTreeLoaderImpl.readFromVFile(StubTreeLoaderImpl.java:119)
at com.intellij.psi.stubs.StubTreeLoaderImpl.readOrBuild(StubTreeLoaderImpl.java:45)
at com.intellij.psi.impl.source.PsiFileImpl.getStubTree(PsiFileImpl.java:630)
at com.intellij.psi.impl.source.PsiFileImpl.getStub(PsiFileImpl.java:594)
at com.illuminatedcloud.intellij.apex.fileType.ApexSourceFile.getTypeDeclarations(SourceFile:82)
at com.illuminatedcloud.intellij.apex.index.ApexTypeDeclarationShortNameIndex.lambda$getTypeDeclarations$0(SourceFile:116)
at com.intellij.psi.impl.PsiCachedValueImpl.doCompute(PsiCachedValueImpl.kt:40)
at com.intellij.util.CachedValueBase.lambda$getValueWithLock$3(CachedValueBase.java:236)
at com.intellij.util.CachedValueBase.computeData(CachedValueBase.java:43)
at com.intellij.util.CachedValueBase.lambda$getValueWithLock$4(CachedValueBase.java:236)
at com.intellij.openapi.util.RecursionManager$1.computePreventingRecursion(RecursionManager.java:111)
at com.intellij.openapi.util.RecursionGuard.doPreventingRecursion(RecursionGuard.java:27)
at com.intellij.openapi.util.RecursionManager.doPreventingRecursion(RecursionManager.java:66)
at com.intellij.util.CachedValueBase.getValueWithLock(CachedValueBase.java:237)
at com.intellij.psi.impl.PsiCachedValueImpl.getValue(PsiCachedValueImpl.kt:34)
at com.intellij.util.CachedValuesManagerImpl.getCachedValue(CachedValuesManagerImpl.java:83)
at com.illuminatedcloud.intellij.apex.index.ApexTypeDeclarationShortNameIndex.getTypeDeclarations(SourceFile:100)
at com.illuminatedcloud.intellij.apex.util.ApexDeclarationSearch.fastFindTypeDeclaration(SourceFile:222)
at com.illuminatedcloud.intellij.apex.util.ApexDeclarationSearch.findTypeDeclaration(SourceFile:137)
at com.illuminatedcloud.intellij.apex.util.ApexDeclarationSearch.findTypeDeclaration(SourceFile:82)
at com.illuminatedcloud.intellij.util.IcPsiDependencyUtil.lambda$static$4(SourceFile:791)
at com.illuminatedcloud.intellij.util.IcPsiDependencyUtil.addTransitiveDependents(SourceFile:600)
at com.illuminatedcloud.intellij.util.IcPsiDependencyUtil.addNewDependents(SourceFile:701)
at com.illuminatedcloud.intellij.util.IcPsiDependencyUtil.addTransitiveDependents(SourceFile:633)
at com.illuminatedcloud.intellij.util.IcPsiDependencyUtil.getTransitiveDependents(SourceFile:305)
at com.illuminatedcloud.intellij.apex.util.ApexDeclarationDependencyUtil$1.compute(SourceFile:76)
at com.intellij.psi.impl.PsiCachedValueImpl$Direct.doCompute(PsiCachedValueImpl.kt:77)
at com.intellij.util.CachedValueBase.lambda$getValueWithLock$3(CachedValueBase.java:236)
at com.intellij.util.CachedValueBase.computeData(CachedValueBase.java:43)
at com.intellij.util.CachedValueBase.lambda$getValueWithLock$4(CachedValueBase.java:236)
at com.intellij.openapi.util.RecursionManager$1.computePreventingRecursion(RecursionManager.java:111)
at com.intellij.openapi.util.RecursionGuard.doPreventingRecursion(RecursionGuard.java:27)
at com.intellij.openapi.util.RecursionManager.doPreventingRecursion(RecursionManager.java:66)
at com.intellij.util.CachedValueBase.getValueWithLock(CachedValueBase.java:237)
at com.intellij.psi.impl.PsiCachedValueImpl$Direct.getValue(PsiCachedValueImpl.kt:81)
at com.intellij.util.CachedValuesManagerImpl.getCachedValue(CachedValuesManagerImpl.java:83)
at com.illuminatedcloud.intellij.apex.util.ApexDeclarationDependencyUtil.getTransitiveDependents(SourceFile:66)
at com.illuminatedcloud.intellij.apex.unitTest.ApexUnitTestRunConfiguration.getDependentTestClassesAndMethods(SourceFile:339)
at com.illuminatedcloud.intellij.apex.unitTest.ApexUnitTestRunConfiguration$2.run(SourceFile:325)
at com.intellij.openapi.progress.impl.CoreProgressManager.startTask(CoreProgressManager.java:477)
at com.intellij.openapi.progress.impl.ProgressManagerImpl.startTask(ProgressManagerImpl.java:133)
at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$runProcessWithProgressSynchronously$9(CoreProgressManager.java:567)
at com.intellij.openapi.progress.impl.ProgressRunner.lambda$new$0(ProgressRunner.java:86)
at com.intellij.openapi.progress.impl.ProgressRunner.lambda$submit$4(ProgressRunner.java:250)
at com.intellij.openapi.progress.ProgressManager.lambda$runProcess$0(ProgressManager.java:100)
at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$runProcess$1(CoreProgressManager.java:221)
at com.intellij.platform.diagnostic.telemetry.helpers.TraceKt.use(trace.kt:46)
at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$runProcess$2(CoreProgressManager.java:220)
at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$executeProcessUnderProgress$13(CoreProgressManager.java:660)
at com.intellij.openapi.progress.impl.CoreProgressManager.registerIndicatorAndRun(CoreProgressManager.java:735)
at com.intellij.openapi.progress.impl.CoreProgressManager.computeUnderProgress(CoreProgressManager.java:691)
at com.intellij.openapi.progress.impl.CoreProgressManager.executeProcessUnderProgress(CoreProgressManager.java:659)
at com.intellij.openapi.progress.impl.ProgressManagerImpl.executeProcessUnderProgress(ProgressManagerImpl.java:79)
at com.intellij.openapi.progress.impl.CoreProgressManager.runProcess(CoreProgressManager.java:202)
at com.intellij.openapi.progress.ProgressManager.runProcess(ProgressManager.java:100)
at com.intellij.openapi.progress.impl.ProgressRunner.lambda$submit$5(ProgressRunner.java:250)
at com.intellij.openapi.progress.impl.ProgressRunner$ProgressRunnable.run(ProgressRunner.java:500)
at com.intellij.util.concurrency.ChildContext$runAsCoroutine$1.invoke(propagation.kt:81)
at com.intellij.util.concurrency.ChildContext$runAsCoroutine$1.invoke(propagation.kt:81)
at com.intellij.util.concurrency.ChildContext.runAsCoroutine(propagation.kt:86)
at com.intellij.util.concurrency.ChildContext.runAsCoroutine(propagation.kt:81)
at com.intellij.openapi.progress.impl.ProgressRunner.lambda$launchTask$18(ProgressRunner.java:466)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(Executors.java:702)
at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(Executors.java:699)
at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1.run(Executors.java:699)
at java.base/java.lang.Thread.run(Thread.java:840)
2024-07-31 12:57:12,731 [85158489] SEVERE - #c.i.u.c.ThreadingAssertions - WebStorm 2024.1.5 Build #WS-241.18034.50
2024-07-31 12:57:12,731 [85158489] SEVERE - #c.i.u.c.ThreadingAssertions - JDK: 17.0.11; VM: OpenJDK 64-Bit Server VM; Vendor: JetBrains s.r.o.
2024-07-31 12:57:12,731 [85158489] SEVERE - #c.i.u.c.ThreadingAssertions - OS: Mac OS X
2024-07-31 12:57:12,731 [85158489] SEVERE - #c.i.u.c.ThreadingAssertions - Plugin to blame: Illuminated Cloud 2 version: 2.3.2.2

‌

[Scott Wells]

Hi, Tony. Yeah, there's no reason that it should take anywhere near that long. My guess is that it's either ending up in a dependency loop (against which it should be safeguarded) or it's thrashing on lots of small dependencies (again, against which it should have a level of caching to avoid processing the same branches repeatedly). Let me think about the best way to get diagnostics for what's going on. I'll most likely add some additional debug logging in this area for tomorrow's build and then you cna provide a debug log of this running so I can see what's happening. I'll let you know.

[Scott Wells]

FWIW, I’ve added quite a bit of debug logging in the transitive dependency/dependent calculation logic, but it won’t make it into tomorrow’s build. I want to spend some time reviewing the logs from a few key use cases, both to refine the way the information is presented to be easier to digest and to see if there are any obvious issues that I need to resolve anyway. The logging – and perhaps some fixes – should be included in next week’s build so that we can get to the bottom of whatever issues still remain.

[Tony White]

Thanks, the only suggestion I can make is that it might be show more readily in relation to methods inside trigger handler classes.

[Scott Wells]

Added more information to #2600 regarding the new diagnostic information that will be included in the next build.

[Scott Wells]

Fixes, optimizations, and logging improvements have been delivered in in 2.3.2.6. If these long runtimes are still present after updating, please enable debug logging for Dependencies/Dependents Calculation and let’s see where the time is going.

[Scott Wells]

Okay, so at least part of the issue here is that when transitive dependents are computed for references that depend on reflection (albeit limited in Apex) or other such dynamic runtime resolution mechanisms, IC2 can include things that you wouldn’t normally want included. Trigger handlers are a perfect example of those because basically all of them have some way of dynamically dispatching from an Apex trigger into the corresponding logic, e.g.

trigger ContactTrigger on Contact (
    before insert,
    before delete,
    after insert,
    after update
)
{
    TriggerHandlerDispatcher.dispatch(ContactTriggerHandler.Factory.class);
}

When gathering dependents of logic that is executed by a trigger handler, pretty much all trigger handlers using that same framework will end up being included because of transitive dependent chains such as:

Adding dependent:
  sourceElements=[
    constructor:ContactTriggerHandler.ContactTriggerHandler(List<SObject>, Map<Id, SObject>) > 
    method:ContactTriggerHandler.Factory.create(List<SObject>, Map<Id, SObject>) > 
    method:TriggerHandlerDispatcher.dispatch(Type) > 
    trigger:<allTriggerHandlerTriggers>
    <allDmlLogicThatResultsInThoseTriggers>
  ]

That’s technically correct but practically not what you’d want. In this case, you’d want it to filter to only the logic that specifically ends up performing DML on Contact SObjects. However, without having runtime data flow information, there’s no obvious way to know which dependent relationships should be included vs. excluded.

What makes the most sense to me is to have a way to exclude certain symbols from the dependents search so that it effectively ends with those symbols when found, e..g, TriggerHandlerDispatcher.dispatch() in the example above. That way dependents of ContactTrigger would still be found but not other usages of dispatch() and therefore other Apex triggers that invoke that method.

What I’m currently thinking is that you’d do this in the code itself as an immediately preceding comment for the declaration that should not be traversed similar to comment-based inspection suppressions, e.g.:

/**
 * Dispatch the current trigger event to a trigger handler created using the specified factory.
 *
 * @param factoryType the class for the trigger handler factory used to create the trigger handler
 *
 * @throws IllegalArgumentException if a null or invalid factory type is provided
 * @throws IllegalStateException if called not in the context of trigger execution or if a trigger handler cannot
 * be created using the provided factory
 */
// no-dependents
public static void dispatch(Type factoryType) { ... }

That way it’s actually part of the code itself in a non-intrusive manner and doesn’t have to exist in some external project-level configuration. I also considered other heuristics such as automatically excluding methods that call Type.newInstance() on a Type that’s provided as a formal parameter, but I’m guessing that there will be plenty of times that such a heuristic will be incorrect, and there are other ways to get a Type for dynamic instantiation, and other ways to instantiate dynamically, e.g., via JSON deserialization.

I’ll play with this a bit and see if it bears fruit locally. If so, I’ll likely attach a test build here so you can see how it looks for you.

[Scott Wells]

Unfortunately what I suggested above won’t work. Without going into too much detail, if I mark the method that ends up pulling in way too much as no-dependents, the things that should be included are also filtered and no dependent tests are ultimately found. Obviously code can be structured in various ways, but in the end this type of reflection-based behavior is simply not a good fit for transitive dependents calculation because the true relationships do not exist until runtime.

FWIW, I did look at the Metadata Dependency API, but its understanding of dependent relationships are both coarse-grained, e.g., at the class-level and not the member-level, and quite naive, e.g., the dependents of the trigger handler in question here is the trigger that instantiates it and nothing else.

I’m going to keep tinkering with some ideas I have on a heuristic filtering approach, and if it starts to look promising, I might add it as an optional behavior.

[Scott Wells]

Okay, after playing with this a bit more – which did lead to one additional small optimization around dependents collection, but not one that would really move the needle in this situation – I’ve decided that there’s not really any way for IC2 to be able to filter dependents 100% automatically when reflection-based dispatch is involved. However, I am thinking there might be a reasonably simple way to allow those who have this situation to give IC2 a bit of additional information to help with filtering.

If IC2 allows the user to specify some simple pattern-based mappings that it can use for filtering, that will help provide a much better signal-to-noise ratio. So, for example, the user should be able to say that when looking at dependents of TriggerHandlerDispatcher.dispatch(), it should only include those for which there are existing entries in the transitive dependency set with a similar/derived name, e.g., (\w+?)TriggerHandler => $1Trigger so that dependent chains that include ContactTriggerHandler would only include dependents via ContactTrigger; AccountTriggerHandler would only include AccountTrigger; etc. This could be expressed – again, directly in the Apex declaration – as something like:

/**
 * Dispatch the current trigger event to a trigger handler created using the specified factory.
 *
 * @param factoryType the class for the trigger handler factory used to create the trigger handler
 *
 * @throws IllegalArgumentException if a null or invalid factory type is provided
 * @throws IllegalStateException if called not in the context of trigger execution or if a trigger handler cannot
 * be created using the provided factory
 */
// map-dependents (\w+?)TriggerHandler => $1Trigger
public static void dispatch(Type factoryType) { ... }

Of course, this only works when there are well-defined naming conventions in place for components that have this type of dynamic/runtime coupling, but that’s a pretty solid best practice anyway.

Thoughts? I’m not sure if there’s really another way to solve this personally, but I’m always open to suggestions.

[Scott Wells]

Okay, I actually implemented the approach described above today – not yet committed to a delivery branch – and it works very well for the TriggerHandler framework that I described above. Looking at the log that you provided, though, I’m not sure how well it would work. I’m not seeing the same pattern where there’s a well-defined class/trigger naming strategy on either side of the dynamic dispatch. Any thoughts on whether this might actually help you or not?

[Scott Wells]

Mulling on this a bit, the issue with reflection-based logic dispatch is really one of a number of potential issues with how IC2 – and really anything that doesn’t use runtime-based control flow information – computes transitive dependencies and dependents. Here are a few that come specifically to mind, but there are certainly others that are similar (and potentially yet others that manifest in other ways):

• Anything that dynamically instantiates an object and then invokes logic through that dynamic instance is going to have issues when trying to determine transitive dependents. Specifically it’s going to find all potential usages of the invoked interface instead of what should be focused usages that would occur at runtime. The TriggerHandlerDispatcher behavior described above is a perfect example of this, and while some kind of mapping hint information like map-dependents can help, even that is obviously not a perfect solution. It might even make sense to allow the user to configure whether or not to traverse any dependent that seems to use Type information for dynamic dispatch, though I’d want to be careful about how IC2 tries to discern that so as not to slow down the normal case for the exceptional case.
• There are also numerous ways that custom code is “connected” by a call to a standard API, e.g., Comparator.compare() is invoked indirectly by List.sort(); Schedulable, Batchable, and Queueable concrete implementations are invoked by the respective standard async Apex registration methods; Iterable implementations can be invoked by Apex for each loops; toString() implementations can be invoked by String concatenation expressions; equals(), and hashCode() implementations can be invoked via various container class methods; etc. Right now IC2’s transitive dependencies/dependents calculators don’t take those more implicit/indirect relationships into account.

I may address some or even all of these, but there will almost certainly still be ones that aren’t properly contemplated given all of the different ways that Salesforce metadata types can interact with one another.

So not really trying to offer a full solution with what I’ve written above, but I did want to capture this brain dump in a more permanent form while it’s rattling around in my head…

[Scott Wells]

Minor additional optimizations/improvements delivered in 2.3.2.7.