TestEngine: Fix some snapshots problems (neo-project#941)

* Fix some snapshots problems

* Clean ut

* Update src/Neo.SmartContract.Testing/TestingApplicationEngine.cs

* clean method

* Update Readme

* Apply suggestions from code review

* Fix method name

* format

* replace interface

Author: shargon

src/Neo.SmartContract.Testing/README.md

@@ -242,7 +242,7 @@ Assert.AreEqual(123, neo.BalanceOf(engine.ValidatorsAddress));
 
 using (ScriptBuilder script = new())
 {
-    script.EmitDynamicCall(neo.Hash, nameof(neo.BalanceOf), engine.ValidatorsAddress);
+    script.EmitDynamicCall(neo.Hash, "balanceOf", engine.ValidatorsAddress);
 
     Assert.AreEqual(123, engine.Execute(script.ToArray()).GetInteger());
 }
@@ -416,7 +416,53 @@ public class CoverageContractTests
 }
 ```
 
-Keep in mind that the coverage is at the instruction level. 
+Keep in mind that the coverage is at the instruction level, but you can also get the project's coverage on the source code using the debug file (`*.nefdbgnfo`) generated by `Neo.Compiler.CSharp`. To do this, you need to compile the project with the `-d` or `--debug` argument, and set up a unit test like the following:
+
+```csharp
+[TestClass]
+public class CoverageContractTests
+{
+    /// <summary>
+    /// Required coverage to be success
+    /// </summary>
+    public static decimal RequiredCoverage { get; set; } = 1M;
+
+    [AssemblyCleanup]
+    public static void EnsureCoverage()
+    {
+        // Join here all of your coverage sources
+
+        var coverage = Nep17ContractTests.Coverage;
+        coverage?.Join(OwnerContractTests.Coverage);
+
+        // Dump coverage to console
+
+        Assert.IsNotNull(coverage, "Coverage can't be null");
+        Console.WriteLine(coverage.Dump());
+
+        // Write basic instruction html coverage
+
+        File.WriteAllText("coverage.instruction.html", coverage.Dump(DumpFormat.Html));
+
+        // Load our debug file
+
+        if (NeoDebugInfo.TryLoad("templates/neocontractnep17/Artifacts/Nep17Contract.nefdbgnfo", out var dbg))
+        {
+            // Write the cobertura format
+
+            File.WriteAllText("coverage.cobertura.xml", coverage.Dump(new CoberturaFormat((coverage, dbg))));
+
+            // Write the report to the specific path
+
+            CoverageReporting.CreateReport("coverage.cobertura.xml", "./coverageReport/");
+        }
+
+        // Ensure that the coverage is more than X% at the end of the tests
+
+        Assert.IsTrue(coverage.CoveredLinesPercentage >= RequiredCoverage, $"Coverage is less than {RequiredCoverage:P2}");
+    }
+}
+```
 
 ### Known limitations
 

src/Neo.SmartContract.Testing/Storage/EngineCheckpoint.cs

@@ -19,7 +19,7 @@ public class EngineCheckpoint
         /// Constructor
         /// </summary>
         /// <param name="snapshot">Snapshot</param>
-        public EngineCheckpoint(SnapshotCache snapshot)
+        public EngineCheckpoint(DataCache snapshot)
         {
             var list = new List<(byte[], byte[])>();
 
@@ -63,7 +63,7 @@ public EngineCheckpoint(Stream stream)
         /// Restore
         /// </summary>
         /// <param name="snapshot">Snapshot</param>
-        public void Restore(SnapshotCache snapshot)
+        public void Restore(DataCache snapshot)
         {
             // Clean snapshot
 

src/Neo.SmartContract.Testing/Storage/EngineStorage.cs

@@ -22,7 +22,7 @@ public class EngineStorage
         /// <summary>
         /// Snapshot
         /// </summary>
-        public SnapshotCache Snapshot { get; private set; }
+        public DataCache Snapshot { get; private set; }
 
         /// <summary>
         /// Return true if native contract are initialized
@@ -33,10 +33,17 @@ public class EngineStorage
         /// Constructor
         /// </summary>
         /// <param name="store">Store</param>
-        public EngineStorage(IStore store)
+        public EngineStorage(IStore store) : this(store, new SnapshotCache(store.GetSnapshot())) { }
+
+        /// <summary>
+        /// Constructor
+        /// </summary>
+        /// <param name="store">Store</param>
+        /// <param name="snapshotCache">Snapshot cache</param>
+        internal EngineStorage(IStore store, DataCache snapshotCache)
         {
             Store = store;
-            Snapshot = new SnapshotCache(Store.GetSnapshot());
+            Snapshot = snapshotCache;
         }
 
         /// <summary>
@@ -52,7 +59,10 @@ public void Commit()
         /// </summary>
         public void Rollback()
         {
-            Snapshot.Dispose();
+            if (Snapshot is IDisposable sp)
+            {
+                sp.Dispose();
+            }
             Snapshot = new SnapshotCache(Store.GetSnapshot());
         }
 

src/Neo.SmartContract.Testing/TestEngine.cs

@@ -305,22 +305,7 @@ public T Deploy<T>(NefFile nef, ContractManifest manifest, object? data = null,
         {
             // Deploy
 
-            if (EnableCoverageCapture)
-            {
-                UInt160 expectedHash = GetDeployHash(nef, manifest);
-
-                if (!Coverage.ContainsKey(expectedHash))
-                {
-                    var coveredContract = new CoveredContract(MethodDetection, expectedHash, new ContractState()
-                    {
-                        Nef = nef,
-                        Hash = expectedHash,
-                        Manifest = manifest
-                    });
-                    Coverage[coveredContract.Hash] = coveredContract;
-                }
-            }
-
+            //UInt160 expectedHash = GetDeployHash(nef, manifest);
             var state = Native.ContractManagement.Deploy(nef.ToArray(), Encoding.UTF8.GetBytes(manifest.ToJson().ToString(false)), data);
 
             if (state is null)

src/Neo.SmartContract.Testing/TestingApplicationEngine.cs

@@ -2,6 +2,7 @@
 using Neo.Persistence;
 using Neo.SmartContract.Native;
 using Neo.SmartContract.Testing.Extensions;
+using Neo.SmartContract.Testing.Storage;
 using Neo.VM;
 using Neo.VM.Types;
 using System;
@@ -146,7 +147,7 @@ private void RecoverCoverage(Instruction instruction)
             {
                 // We need the contract state without pay gas
 
-                var state = NativeContract.ContractManagement.GetContract(Engine.Storage.Snapshot, contractHash);
+                var state = NativeContract.ContractManagement.GetContract(Snapshot, contractHash);
 
                 coveredContract = new(Engine.MethodDetection, contractHash, state);
                 Engine.Coverage[contractHash] = coveredContract;
@@ -208,9 +209,30 @@ protected override void OnSysCall(InteropDescriptor descriptor)
 
                     // Invoke
 
-                    var hasReturnValue = customMock.Method.ReturnType != typeof(void);
-                    var returnValue = customMock.Method.Invoke(customMock.Contract, parameters);
-                    if (hasReturnValue)
+                    object? returnValue;
+                    EngineStorage backup = Engine.Storage;
+
+                    try
+                    {
+                        // We need to switch the Engine's snapshot in case
+                        // that a mock want to query the storage, it's not committed
+
+                        Engine.Storage = new EngineStorage(backup.Store, Snapshot);
+
+                        // Invoke snapshot
+
+                        returnValue = customMock.Method.Invoke(customMock.Contract, parameters);
+                    }
+                    catch
+                    {
+                        throw;
+                    }
+                    finally
+                    {
+                        Engine.Storage = backup;
+                    }
+
+                    if (customMock.Method.ReturnType != typeof(void))
                         Push(Convert(returnValue));
                     else
                         Push(StackItem.Null);

src/Neo.SmartContract.Testing/TestingStandards/Nep17Tests.cs

@@ -204,22 +204,37 @@ public virtual void TestTransfer()
 
         UInt160? calledFrom = null;
         BigInteger? calledAmount = null;
-        byte[]? calledData = null;
+        UInt160? calledData = null;
 
         var mock = Engine.Deploy<onNEP17PaymentContract>(NefFile, manifest.ToJson().ToString(), null, m =>
          {
              m
              .Setup(s => s.onNEP17Payment(It.IsAny<UInt160>(), It.IsAny<BigInteger>(), It.IsAny<object>()))
              .Callback(new InvocationAction((i) =>
              {
+                 // Set variables
+
+                 var me = new UInt160((i.Arguments[2] as ByteString)!.GetSpan().ToArray());
                  calledFrom = i.Arguments[0] as UInt160;
                  calledAmount = (BigInteger)i.Arguments[1];
-                 calledData = (i.Arguments[2] as ByteString)!.GetSpan().ToArray();
+
+                 // Ensure the balance
+
+                 Assert.AreEqual(3, Contract.BalanceOf(me));
 
                  // Ensure the event was called
 
-                 var me = new UInt160(calledData);
                  AssertTransferEvent(Alice.Account, me, calledAmount);
+
+                 // Return the money back
+
+                 Engine.SetTransactionSigners(me);
+                 Assert.IsTrue(Contract.Transfer(me, calledFrom, calledAmount));
+                 AssertTransferEvent(me, Alice.Account, calledAmount);
+
+                 // Set success flag
+
+                 calledData = me;
              }));
          });
 
@@ -229,14 +244,9 @@ public virtual void TestTransfer()
         Assert.IsTrue(Contract.Transfer(Alice.Account, mock.Hash, 3, mock.Hash.ToArray()));
 
         Assert.AreEqual(Alice.Account, calledFrom);
-        Assert.AreEqual(mock.Hash, new UInt160(calledData));
+        Assert.AreEqual(mock.Hash, calledData);
         Assert.AreEqual(3, calledAmount);
-
-        // Return the money back
-
-        Engine.SetTransactionSigners(mock);
-        Assert.IsTrue(Contract.Transfer(mock.Hash, calledFrom, calledAmount));
-        AssertTransferEvent(mock.Hash, Alice.Account, calledAmount);
+        Assert.AreEqual(0, Contract.BalanceOf(mock.Hash));
     }
 
     #endregion

tests/Neo.SmartContract.Template.UnitTests/templates/neocontractnep17/CoverageContractTests.cs

@@ -15,24 +15,35 @@ public class CoverageContractTests
         [AssemblyCleanup]
         public static void EnsureCoverage()
         {
-            // Join here all of your Coverage sources
+            // Join here all of your coverage sources
 
             var coverage = Nep17ContractTests.Coverage;
             coverage?.Join(OwnerContractTests.Coverage);
 
-            // Ensure that the coverage is more than X% at the end of the tests
+            // Dump coverage to console
 
-            Assert.IsNotNull(coverage);
+            Assert.IsNotNull(coverage, "Coverage can't be null");
             Console.WriteLine(coverage.Dump());
 
-            File.WriteAllText("instruction-coverage.html", coverage.Dump(DumpFormat.Html));
+            // Write basic instruction html coverage
+
+            File.WriteAllText("coverage.instruction.html", coverage.Dump(DumpFormat.Html));
+
+            // Load our debug file
 
             if (NeoDebugInfo.TryLoad("templates/neocontractnep17/Artifacts/Nep17Contract.nefdbgnfo", out var dbg))
             {
+                // Write the cobertura format
+
                 File.WriteAllText("coverage.cobertura.xml", coverage.Dump(new CoberturaFormat((coverage, dbg))));
+
+                // Write the report to the specific path
+
                 CoverageReporting.CreateReport("coverage.cobertura.xml", "./coverageReport/");
             }
 
+            // Ensure that the coverage is more than X% at the end of the tests
+
             Assert.IsTrue(coverage.CoveredLinesPercentage >= RequiredCoverage, $"Coverage is less than {RequiredCoverage:P2}");
         }
     }