A template (or util) method can provide basic contexts, and the developers just
need to override the parts they need (of course they should know which parts
they need).
In the constructor-way, the developer should know what instances they will use,
create them, and pass them to the constructor;
while in the context-way, the developer should know what fields they will use,
create them, and override them in the context.
I do not think they are essentially different.
The problem is, for high level structures like DataRegion, they will have many
many more fields because their children use many different ones, and the
constructor will be very long for them.
Certainly, you may further migrate the constructor-way to the builder-way,
which may solve the problem. However, it requires much more extra work.
I am not particularly fond of either one and I just want to point out the
problems, hoping someone may give a perfect solution.
If none is given, I am happy to accept either one.
Tian Jiang
---- Replied Message ----
| From | Christofer Dutz<christofer.d...@c-ware.de> |
| Date | 7/18/2024 15:39 |
| To | dev@iotdb.apache.org<dev@iotdb.apache.org> |
| Subject | AW: AW: Possible pattern for reducing the negative effects of using
singletons and improving testability |
However, then you need to construct these contexts with loads of parameters for
unit tests … I personally don’t think passing in explicitly what you need into
a constructor is bad.
So, if you have this service that you want to test: How can you tell the
developer writing the test which services are needed and therefore need to be
mocked in the context?
I would strongly opt for explicit dependency injection in the constructor and
not using some context.
Chris
Von: Tian Jiang <jt2594...@163.com>
Datum: Donnerstag, 18. Juli 2024 um 09:27
An: dev <dev@iotdb.apache.org>
Betreff: Re: AW: Possible pattern for reducing the negative effects of using
singletons and improving testability
Yes, maybe we could have leveled context like DataNodeContext,
DataRegionContext, TsFileProcessorContext... and put the former singletons in
these contexts.
This may avoid constructors with dozens of parameters.
Tian Jiang
| |
Tian Jiang
|
|
jt2594...@163.com
|
---- Replied Message ----
| From | Christofer Dutz<christofer.d...@c-ware.de> |
| Date | 7/18/2024 15:22 |
| To | dev@iotdb.apache.org<dev@iotdb.apache.org> |
| Subject | AW: Possible pattern for reducing the negative effects of using
singletons and improving testability |
Ideally,
we could inspect the usages of the constructors using the singletons and apply
the same pattern there. Once all usages have been migrated that way, we could
even eliminate the Singleton-using constructor. In the end getting a system for
which we can finally write sensible unit-tests again and don’t need to rely on
Integration tests.
Integration tests that have problems running in paralell, while unit-tests can
run perfectly in paralell.
Chris
Von: Christofer Dutz <christofer.d...@c-ware.de>
Datum: Donnerstag, 18. Juli 2024 um 09:03
An: dev@iotdb.apache.org <dev@iotdb.apache.org>
Betreff: AW: Possible pattern for reducing the negative effects of using
singletons and improving testability
Ok …
So, as there was no objection here, I’ll trat that as consensus and I’ll just
do this for every component that I touch and hereby need to write tests for.
Chris
Von: Christofer Dutz <christofer.d...@c-ware.de>
Datum: Montag, 15. Juli 2024 um 11:49
An: dev@iotdb.apache.org <dev@iotdb.apache.org>
Betreff: Possible pattern for reducing the negative effects of using singletons
and improving testability
Hi all,
I am currently working on some functions and when writing tests for these, I
had problems sensibly testing things, as the singletons were preventing that.
As I mentioned before … I consider the usage of singletons the probably worst
design decision done in IoTDBs core. Even if it simplifies “composition”, it
really makes everything else more complicated.
Knowing how deeply rooted in IoTDB the use of singletons is, I always thought
this would be a never-ending story.
But I just recently had an idea of how we could probably find a way to get the
testability that we need without having to give up on singletons.
Possibly this path could also provide a path to getting rid of singletons over
time.
As I was just working on DataNodeServerCommandLine I did an experiment with
that, and it worked nicely.
So, I searched for all singletons used in the component and defined private
final member variables for each of them.
In the no-args constructor I initialize these via the singletons, but I also
add a second constructor, that allows passing in explicit versions.
private final ConfigNodeInfo configNodeInfo;
private final IClientManager<ConfigRegionId, ConfigNodeClient>
configNodeClientManager;
private final DataNode dataNode;
/** Default constructor using the singletons for initializing the relationship.
*/
public DataNodeServerCommandLine() {
configNodeInfo = ConfigNodeInfo.getInstance();
configNodeClientManager = ConfigNodeClientManager.getInstance();
dataNode = DataNode.getInstance();
}
/**
* Additional constructor allowing injection of custom instances (mainly for
testing)
*
* @param configNodeInfo config node info
* @param configNodeClientManager config node client manager
* @param dataNode data node
*/
public DataNodeServerCommandLine(
ConfigNodeInfo configNodeInfo,
IClientManager<ConfigRegionId, ConfigNodeClient> configNodeClientManager,
DataNode dataNode) {
this.configNodeInfo = configNodeInfo;
this.configNodeClientManager = configNodeClientManager;
this.dataNode = dataNode;
}
With this was I now able to write real unit-tests for the component, without
having to require writing an Integration-test which costs a lot more resources
and which makes implementing unhappy-path tests really challenging.
With this approach, I think we can really improve testability in IoTDB …
considering a class test coverage of only 24% and a method coverage of just 19%
in the data node, we really need this.
What do you all think?
Chris
