archaius icon indicating copy to clipboard operation
archaius copied to clipboard

Refactor PropertyTest.test() to Improve Test Experience

Open Codegass opened this issue 11 months ago • 0 comments

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.

Codegass avatar Mar 18 '24 08:03 Codegass