Test Reload_Detector through the language server

[GregoryTravis]

Write a test that checks that the Managed_Resource cache-clearing method in Reload_Detector works correctly, without using a fake trigger, as the test in Fetch_Spec.enso does. Original discussion

Options are:

• Add a builtin that allows scheduleFinalizationOfSystemReferences to be called from Enso
• Write a test like RuntimeExecutionEnvironmentTest.scala
• Write a test like ManagedResourceTest.java

[JaroslavTulach]

Add a builtin that allows scheduleFinalizationOfSystemReferences to be called from Enso

No, I am against adding builtins just to artificially increase test coverage.

[JaroslavTulach]

Just to repeat myself: Logic of Managed_Resource.register ref on_finalize Boolean.True is unit tested at

enso/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/runtime/ManagedResourceTest.java

Line 96 in cc0b020

ensoCtx.getResourceManager().scheduleFinalizationOfSystemReferences();

We have a unit test of Managed_Resource and another unit test for ReloadDetector, then I believe we have 100% coverage and there is no need to seek more coverage. At least I would personally wait for first bug report about something being broken. Just my 2 Kč.

had the tests pass but the feature fail to work

If one has too... then it is up to you guys... just no builtins or anything that would be visible (and I don't count ## PRIVATE comments as hiding anything) for Enso users.

[radeusgd]

had the tests pass but the feature fail to work

Yes we already had a bug in there. There's no report because we managed to find it before the PR introducing it was even merged. But that suggests that the test coverage is not enough for this feature - as @GregoryTravis noted - all unit tests were passing but the feature was not working.

If one has too... then it is up to you guys... just no builtins or anything that would be visible (and I don't count ## PRIVATE comments as hiding anything) for Enso users.

@JaroslavTulach What about a private builtin? Only testable in Base_Internal_Tests which has the private access enabled? It would be far easier than instrumenting a language server scenario.

[GregoryTravis]

@JaroslavTulach I respect your opinion about not adding builtins -- so the alternative for this is a Scala test, I think. Can you point to a test that I could base this on? One that can trigger the real resource finalization, and examine the effect on a real Enso value.

[JaroslavTulach]

I believe you can make a copy of ManagedResourceTest:

enso/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/runtime/ManagedResourceTest.java

Line 22 in 9f0c0c0

public class ManagedResourceTest {

I believe it tests everything:

• scheduleFinalizationOfSystemReferences
• assertGC
• Managed_Resource

It should be a great base for your test.

[radeusgd]

btw. reading the #11673 PR I think it would also be good to add a test that ensures that clicking the refresh button once, ensures that reload happens only once. To avoid a situation where clicking refresh button would cause caches to be effectively disabled.

Thus the test should not be testing Reload_Detector (which is missing the 'reset' logic) but the ReloadDetector.java.