[coordinator-test] Remove the need for a real VC

[liran-funaro]

Type of change

• Improvement (improvement to code, performance, etc)
• Test update

Description

• Add support for injecting predefined TX status
• Keep tx execution order for validation in tests
• Remove the use of a real VC and DB in the coordinator testing

Related issues

• resolves [coordinator test] Enable verify keys in the mock VC #187
• resolves [coordinator test] Use mock VC for all coordinator tests #93

[coveralls]

coverage: 91.656% (+0.04%) from 91.62% — liran-funaro:coordinator-test-no-db into hyperledger:main

[cendhu]

I am fine with this change but add unit-test for this mock vcservice as it has some core logic such as MVCC.

[cendhu]

@liran-funaro Not sure whether you missed my comment. Please add unit-test for this mock vcservice.

[liran-funaro]

@cendhu Added tests

[cendhu]

We need to address two issues with this approach:

1. Logic Duplication & Correctness: We are duplicating the core logic of vcservice here, which makes me question if the trade-off is worth it. It already allowed two bugs to slip past our coordinator and mock tests. Since correctness is strictly more important than reducing test runtime, we should evaluate these changes carefully to avoid introducing hard-to-detect bugs.
2. Future Maintenance: When we implement snapshots and checkpoints, I'm concerned we will have to add even more complexity to this mock.

As a general rule, mocks should be kept as simple (or 'dumb') as possible. While I supported this change earlier, I am skeptical about it now.

[liran-funaro]

@cendhu I understand your concern. The motivation is not performance, but test encapsulation.
We don't want VC issues to fail the coordinator.
The alternative is to inject responses manually without really doing the validation. Also, injecting the world-state manually.
This will reduce significantly the mock complexity and can arguably make the tests more explicit as the test will explicitly set the VC behaviour.

[cendhu]

The alternative is to inject responses manually without really doing the validation. Also, injecting the world-state manually. This will reduce significantly the mock complexity and can arguably make the tests more explicit as the test will explicitly set the VC behaviour.

Yes, this sounds interesting and similar to how most mock frameworks work. How can we approach this?

[liran-funaro]

@cendhu I modified the PR according to the discussion in this PR

[cendhu]

LGTM. This read much simpler. Thanks for making this change.