Initialize stdlib values of Test contract lazily

[m-Peter]

Closes #2215

Description

Lazily instantiated stdlib values related to Test contract.
Since we have an accepted proposal (onflow/developer-grants#148), regarding the Cadence testing framework, we thought it would be a good first issue to tackle, in order to get acquainted with the codebase 🙏

---

• Targeted PR against master branch
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work
• Code follows the standards mentioned here
• Updated relevant documentation
• Re-reviewed Files changed in the Github PR explorer
• Added appropriate labels

[codecov]

Codecov Report

Merging #2403 (ae9016e) into master (d7f3648) will decrease coverage by 0.01%.
The diff coverage is 66.25%.

@@            Coverage Diff             @@
##           master    #2403      +/-   ##
==========================================
- Coverage   78.29%   78.29%   -0.01%     
==========================================
  Files         325      327       +2     
  Lines       72465    72697     +232     
==========================================
+ Hits        56738    56919     +181     
- Misses      13644    13687      +43     
- Partials     2083     2091       +8     

Flag	Coverage Δ
unittests	78.29% <66.25%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
runtime/stdlib/test_emulatorbackend.go	29.68% <29.68%> (ø)
runtime/stdlib/test.go	30.27% <41.17%> (-23.96%)	⬇️
runtime/stdlib/test_contract.go	83.53% <83.53%> (ø)

... and 9 files with indirect coverage changes

[SupunS]

Nice! Thank you for adding this!

[turbolent]

Great start 👏

[bluesign]

why this is in cadence ? shouldn't be in https://github.com/onflow/cadence-tools/test ?

[SupunS]

why this is in cadence ?

To avoid circular dependency. Because the test module depends on cadence already (at go level). Moving this to test would cause cadence to also depends on the test module, creating the cycle.
We could eventually move all standard-libs outside of cadence, but that going to need some refactoring and a dependency injection mechanism, so that stdlibs can be provided to the typechecker/runtime, without a hard dependency.

[bluesign]

why this is in cadence ?

To avoid circular dependency. Because the test module depends on cadence already (at go level). Moving this to test would cause cadence to also depends on the test module, creating the cycle. We could eventually move all standard-libs outside of cadence, but that going to need some refactoring and a dependency injection mechanism, so that stdlibs can be provided to the typechecker/runtime, without a hard dependency.

can't we simple expose Test from test runner? If test depends on cadence, but cadence doesn't depend on test.

[turbolent]

@bluesign The current idea is that Cadence itself exposes a test framework in its standard library. cadence-tools/test is just one possible concrete test runner that uses the FVM/Emulator. However, one could imagine other implementations for Flow, and even implementations for other chains.

That is similar to e.g. Rust having testing support in the language and the cargo test runner tool.

That does not mean that it has to stay that way of course, we could consider refactoring the test framework out of Cadence. What do you think would be the advantages of that?

[SupunS]

can't we simple expose Test from test runner? If test depends on cadence, but cadence doesn't depend on test.

Cadence needs to know about the Test contract (i.e: stdlib) statically. So moving it to the test module will create the cycle.

[bluesign]

That does not mean that it has to stay that way of course, we could consider refactoring the test framework out of Cadence. What do you think would be the advantages of that?

I don't know if any advantage tbh; but I always thought, host can expose stuff ( like flow-go was exposing some account freezing before ) to Cadence. As we don't use test on mainnet/testnet, I thought that would be the way. Even I was thinking if it is possible to bridge js-cadence like this. ( like webassembly bridging js - go etc )

[turbolent]

@bluesign Sounds good. Maybe open a new issue for that, as this PR only really is an optimization of the existing code

[m-Peter]

@turbolent If this is in good shape, could we get it merged? I want to open another PR (for onflow/cadence-tools#108), however, I have branched-off of this one, to avoid conflicts 🙏

[turbolent]

@m-Peter Nice work, I'll review tomorrow

[turbolent]

Thanks Ardit for continuing to improve this!

It would be nice if this could be refactored more to avoid all the global variables.

In the current version, the main initialization function, initTest, triggers the initialization of all those globals in an imperative way, which is then often followed by a use of the global for further initialization, and later to also construct a value. Between the globals there are many inter-dependencies.

For example, initEmulatorBackendType initializes the global emulatorBackendType, but it actually depends on the function globals, i.e. initEmulatorBackendFunctions having being called and e.g. the global emulatorBackendExecuteScriptFunctionType having been initialized. Finally, emulatorBackendType exists because it is needed in newEmulatorBackend.

Instead, maybe the initialization could be done in a more "functional" way without side-effects / global initialization. This could be done e.g. by introducing a type TestEmulatorBackendType, which holds the composite type and all function types. The type is then returned from initEmulatorBackendType, instead of it mutating a global. initTest could then be the place where the whole group is initialized.

I pushed a refactor in 25279e6 illustrating the idea.
I'll try to refactor the remainder, mainly, the Test contract code itself.

[m-Peter]

Thanks for the heavy lifting 🙌 💯
This does look way much better now 🎉
I guess there is no remainder here for refactoring, right?

[turbolent]

@m-Peter No, that should be all now. We'll need to update some downstream dependencies, but the required changes are very small.

@dsainati1 @SupunS Could you please review?

[m-Peter]

@turbolent I can update the downstream dependencies, as soon as there's a release. I have to update the cadence-tools/test repo, because I introduced another breaking change, regarding the runtime.Config. Ping me any time, to do so 🙏