archaius
archaius copied to clipboard
Refactor PropertyTest.test() to Improve Test Experience
Description
When I am running the PropertyTest
I notice that there is test case named as test
. It does not have a proper testing name and contains two different testing targets together, which might be clearer and more maintainable if split. Currently, it initializes a service, verifies default values, changes properties, and checks the results in one go.
public void test() throws ConfigException {
SettableConfig config = new DefaultSettableConfig();
DefaultPropertyFactory factory = DefaultPropertyFactory.from(config);
MyService service = new MyService(factory);
assertEquals(1, (int)service.value.get());
assertEquals(2, (int)service.value2.get());
config.setProperty("foo", "123");
assertEquals(123, (int)service.value.get());
assertEquals(123, (int)service.value2.get());
// setValue() is called once when we init to 1 and twice when we set foo to 123.
assertEquals(1, service.setValueCallsCounter.get());
}
Suggested Refactoring
Splitting into two tests could improve clarity, they will be rename as testServiceInitializationWithDefaultProperties
and testPropertyValuesUpdateAndEffect
@Test
public void testServiceInitializationWithDefaultProperties() throws ConfigException {
SettableConfig config = new DefaultSettableConfig();
DefaultPropertyFactory factory = DefaultPropertyFactory.from(config);
MyService service = new MyService(factory);
// Verify initial property values
assertEquals(1, (int) service.value.get());
assertEquals(2, (int) service.value2.get());
// Verify setValue() was called once for each initialization
assertEquals(0, service.setValueCallsCounter.get());
}
@Test
public void testPropertyValuesUpdateAndEffect() throws ConfigException {
SettableConfig config = new DefaultSettableConfig();
DefaultPropertyFactory factory = DefaultPropertyFactory.from(config);
MyService service = new MyService(factory);
// Setting up properties
config.setProperty("foo", "123");
// Assertions after properties update
assertEquals(123, (int)service.value.get());
assertEquals(123, (int)service.value2.get());
// setValue() is called once for each property update
assertEquals(1, service.setValueCallsCounter.get());
}
Benefits
- Clarity: Each test has a clear name focuses on a single aspect, making their purpose and outcomes clearer.
- Maintainability: Easier to update or fix tests related to specific behaviors.
If you don't mind, I am willing to submit the PR for the suggestion.