Hello everyone,
I hope you all had a nice christmas time.
I started to refactor several parts of the Freenet Code to make the it
easier to test and understand the different parts of it in isolation.
Before I continue doing so, I would like to know your thoughts on my
approach and whether you would actually welcome these changes. You can
find the latest version of the partially refactored code on Github [1].
I recently rebased my code so that all commits fit the current state of
the next-branch.
Essentially, I did the following three changes to the Node-,
NodeCrypto-,PeerManager-,UdpSocketHandler-,NodeIPPortDetector- and
IPAddressDetector-class:
1. Create corresponding Interfaces for each class. This allows us to
replace actual class instances (e.g. of the Node class) with a
test-specific dummy implementation. This dummy can then be given as
parameter to a constructor.
2. Encapsulate instantiation of class members within protected methods.
By overriding these protected methods, we can replace class members with
test-specific dummy implementations.
3. Create getter-Methods to access class members. Many classes in
Freenet hold a reference to a Node-instance and some directly access its
members. Besides the fact that this is not a good practice, it kept me
from creating an interface for the node class.
One obstacle with point (1) was that there are several cases where a
class calls a non-public method from another class in the same package.
To illustrate my changes, consider the following three example classes:
package example.A;
class MyClass {
boolean getBool() { ... }
public String getString() { ... }
}
package example.A;
class SamePackageClass {
public SamePackageClass(MyClass ref) {
if (ref.getBool()) ...
}
}
package example.B;
class OtherPackageClass {
public OtherPackageClass(MyClass ref) {
process(ref.getString());
}
}
Because SamePackageClass is within the same package as MyClass, the
getBool-method is visible to the former wheras only getString() is
visible for the OtherPackageClass. After my refactoring, we now have the
following 2 interfaces:
package example.A;
public interface MyClass {
String getString();
}
package example.A;
interface ProtectedMyClass extends MyClass {
boolean getBool();
}
Please note that the first interface is a public interface while the
second interface is package-local. The three example classes now look as
follows:
package example.A;
class MyClassImpl implements ProtectedMyClass {
public boolean getBool() { ... }
public String getString() { ... }
}
package example.A;
class SamePackageClass {
public SamePackageClass(ProtectedMyClass ref) {
if (ref.getBool()) ...
}
}
package example.B;
class OtherPackageClass {
public OtherPackageClass(MyClass ref) {
process(ref.getString());
}
}
The getBool-Method of the MyClassImpl-class is now public. However, if
instances of this class are only accessed via the interfaces it is only
visible to classes that use the ProtectedMyClass interface. This way, we
maintain the package-local view for the different methods. What do you
think about this solution?
Given the above classes, an example for a point (2)-refactoring is as
follows:
class AnotherClass {
MyClass ref;
public AnotherClass(int param) {
ref = newMyClass(param);
}
protected MyClass newMyClass(int param) {
return new MyClassImpl(param);
}
}
Instead of calling "new MyClassImpl(param)" directly in the constructor,
I use a protected method newMyClass. When writing a test case, I can now
easily test AnotherClass without needing an instance of MyClassImpl. To
do so, I create a minimal class MyClassStub that implements the
MyClass-interface and derive the following class:
class InstrumentedAnotherClass extends AnotherClass {
public InstrumentedAnotherClass(int param) {
super(param);
}
@Override
protected MyClass newMyClass(int param) {
return new MyClassStub();
}
}
As a proof of concept for this approach, I created some initial unit
tests for the NodeCrypto[2]- and NodeIPPortDetector-class[3].
To me, two points that are still open when it comes to tests are:
- Testing of time-based conditions: To test functionalities such as
rate-limiting of requests, the calls to System.currentTimeMillis()
probably need to be replaced by a call to a non-system-interface with a
similar method, whose implementation can then be adapted to test cases.
- Concurrency: I didn't yet spend much time thinking about how classes
can be tested that wait for other threads to operate (e.g. location
swapping) but I am optimistic that this can be done.
Kind regards,
Martin
[1] https://github.com/yadevel/fred/commits/testing-refactoring
[2]
https://github.com/yadevel/fred/blob/testing-refactoring/test/freenet/node/NodeCryptoTest.java
[3]
https://github.com/yadevel/fred/blob/testing-refactoring/test/freenet/node/NodeIPPortDetectorTest.java
--
Martin Byrenheid
Scientific Assistant
Technische Universität Dresden
Faculty of Computer Science
Institute of System Architecture
Chair of Privacy and Data Security
01062 Dresden
Tel.: +49 351 463 38235
GPG : F144 FBCD F0C2 257A 87ED
CFDE 24EB E684 D0BE E785
