Skip to content

Reverting Snapshots commits at the end of UTs#1343

Closed
vncoelho wants to merge 4 commits intomasterfrom
reverting-snapshot-on-tests
Closed

Reverting Snapshots commits at the end of UTs#1343
vncoelho wants to merge 4 commits intomasterfrom
reverting-snapshot-on-tests

Conversation

@vncoelho
Copy link
Copy Markdown
Member

@vncoelho vncoelho commented Dec 9, 2019

There are still other places to be reverted,

@ixje @shargon @erikzhang,

Take at look at this example and, if correct, we may create a function on Tests Utils, for avoiding duplicating the code on every case it happens.

image

@erikzhang
Copy link
Copy Markdown
Member

I think the problem is Blockchain.Singleton. We should try to remove Blockchain.Singleton and use dependency injection. Then all the tests are isolated.

@Tommo-L
Copy link
Copy Markdown
Contributor

Tommo-L commented Dec 10, 2019

@rodoufu #1042

@ixje
Copy link
Copy Markdown
Contributor

ixje commented Dec 10, 2019

I think the problem is with

[TestInitialize]
public void TestSetup()
{
TestBlockchain.InitializeMockNeoSystem();
}

I think it is correct to use this in TestInitialize, the problem is that this doesn't seem to create a new fresh Blockchain instance. Each call (and thus each test) should get a new clean instance. It's probably best to use the In-Memory blockchain if that's already merged somewhere, otherwise you'll probably have to add [TestCleanup].

@vncoelho
Copy link
Copy Markdown
Member Author

I agree, @erikzhang, that DI could solve this in a more beautiful way.

However, even nowadays, it is possible to handle that. We should not have tests that are finishing with a modified status different than the original.

Thus, perhaps, until DI is not implemented we should fix the tests that are doing such behavior.

@rodoufu
Copy link
Copy Markdown

rodoufu commented Dec 11, 2019

@rodoufu #1042

The DI may help to improve this kind of initialization for the tests.
But it may be also necessary to mock some stuff, probably we are going to register the components on the container differently for some tests.

@vncoelho vncoelho closed this May 20, 2020
@vncoelho vncoelho deleted the reverting-snapshot-on-tests branch May 20, 2020 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants