hashgraph / hedera-services

Crypto, token, consensus, file, and smart contract services for the Hedera public ledger
Apache License 2.0
266 stars 119 forks source link

Modularization: Resolve platform testing code dependencies #5162

Open swirlds-automation opened 1 year ago

swirlds-automation commented 1 year ago

Problem

The core of this problem is that there is test code inside the swirlds-platform-core module and inside the swirlds-platform-test module that is needed to write unit tests, but it is impossible to import both with the current schema.

Specifically, in order to unit test the hashgraph class, we need code from swirlds-platform-core such as DummyPlatform and DummyEventFlow, and we need the event generation utilities defined in swirlds-platform-test.

We cannot have swirlds-platform-core depend on swirlds-platform-test under any circumstances. And it is not obvious to me how to import modules defined in the test directory under swirlds-platform-core from an outside scope.

Proposal 1: Finish migration to test module

Move the remainder of the test code out of swirlds-platform-core into swirlds-platform-test.

Unfortunately, this has the side effect of making a large number of classes and methods public in swirlds-platform-core.

Proposal 2: Copy and Paste core -> test

Copy and paste the required test utilities out of swirlds-platform-core into swirlds-platform-test. Will still require some classes to be made public.

I personally don't like this approach because multiple versions of the same code will migrate apart and be a pain to merge in the future.

Proposal 3: Copy and Paste test -> core

Copy and paste the event generation utilities into swirlds-platform-core and write the new unit tests there.

I think this is even worse than proposal 2. To do this would require thousands of lines to be copied and maintained separately.

Proposal 4: pull testing code back into platform core

Take all of the platform testing code that is currently in the testing module and move it back to swirlds-platform-core.

swirlds-automation commented 1 year ago

Proposal 1 can only be done IFF all platform core classes are refactored. Otherwise, we will be making an unacceptable number of internal classes available to the general public and polluting our public API.
author:nathanklick, createdAt:2021-01-22T17:16:06Z, updatedAt=2021-01-22T17:16:06Z

swirlds-automation commented 1 year ago

Part of Epic https://github.com/swirlds/swirlds-platform/issues/5004 author:hendrikebbers, createdAt:2022-09-15T20:21:02Z, updatedAt=2022-09-15T20:21:02Z

swirlds-automation commented 1 year ago

migrated from: url=https://github.com/swirlds/swirlds-platform/issues/2898 author:cody-littley, #:2898, createdAt:2021-01-22T15:31:47Z, updatedAt=2023-02-17T19:56:52Z labels=Migration:Base