Restructuring: The Art of Replacing the Wheels of a Running Train

This is a translation from the original article: https://toss.tech/article/restructuring

Restructuring vs. Refactoring

In software engineering, refactoring can be defined as following:

Restructuring the code without changing its results.

If refactoring was done properly, there should be no functional change. The test code results should be the same. As a result, we can ensure that all previous features are working well through the test code execution result.

This 'refactoring resistance' is important for us to trust the test code. If the test fails without having changed the feature, it's called a false report. Even in one test case, if a 'false report' is made, the entire test code should be suspected. That's why in my project I improved refactoring resistance first.

Identifying refactoring resistance

I have identified three aspects that reduce the refactoring resistance.

1. Relying on the order in which the test cases are executed
2. Relying on internal implementations
3. Relying on external dependencies

1. Relying on the order in which the test cases are executed

When the test cases are referencing to the same mock object or share a state, then they depend on the execution order. This might result in test failures. In particular, it frequently happens in environments running asynchronously. As an example, if you are directly referring to the session storage, the cleanup may not be done properly and unintended behavior may occur.

The reason for this failure is the test case not running independently. Therefore, I limited the scope of the mock object and removed all the states being shared. It's an easy problem, but you may often encounter it when writing test code.

Actions to be taken

• The test code result should not depend on its execution order
• Instead of sharing the object, let's use a builder function that returns the object needed.

2. Relying on internal implementations

You've probably heard about the relationship between functionality and implementation, interface and detail. Relying on implementation in the test code that tests the functionality will cause problems. There might be no big problem at the time of writing the code, but it might become when maintaining the production code.

Let's test a component called Page like the following:

function Page() {
  const data = useUserResource();

  return <div>{data.name}</div>
}

We may write a test code that tests if this component renders name:

it('Page render name', () => {
  render(<Page />);

  expect(getByText('toss')).toBeInDocument();
});

Here, we'll use a mocked object in order to define the value returned by useUserResource.

const mockUseUserResource = mock('./useUserResource');

it('Page renders name', () => {
  mockUseUserResource.returnValue({
    name: 'toss',
  });
  render(<Page />);

  expect(getByText('toss')).toBeInDocument();
});

Now, this test code relies on the component's implementation. Let's see why.

We want to test if the Page component renders the value name well. However, if you change the 'path of file' where the hook called useUserResource is defined, the test will be broken. The file path is not important information from the Page component's point of view. It's mere detail that should be ignored by the test code.

Actions to be taken

• Let's remove the internal implementation of the component shown in the test code.
• Let's remove implicit dependencies.

3. Relying on external dependencies

This is the most discussed topic in my team, and we first talked about how to define the boundary of "external" from the standpoint of our product.

The network layer for invoking server APIs could easily be defined as external dependencies, but libraries such as React were hard to be defined. This is because if it is defined as external dependency, it was necessary to examine whether the cost of maintanence.

What about a logging module to track user behavior? Is this code, which is just a module, an external dependency? Are web APIs like Local Storage and Session Storage external dependencies?

Action 1. Identify the external dependencies

You might have heard of Clean Architecture diagram:

https://blog.cleancoder.com/uncle-bob/2012/08/13/the-clean-architecture.html

Some people say, "Front-end applications and clean architecture don't fit." Clean architecture separates layers with domain logic at the center, and you have to understand why you divide them to understand clean architecture. In my team, we didn't just follow the design presented by clean architecture, but we divided these 'layers'.

We thought about what would be pushed out if we constructed a use case around the domain model. As a result, the following are the dependencies we identified based on the boundaries we defined:

1. API Client - this is the network client that communicates with the server
2. Storage - such as local storage and session storage
3. Bridge - this offers the interface in charge of communicating between different windows
4. Logging - in charge of logging the flow or the user's actions
5. Runtime - for example, the browser history, user agent, etc.

Action 2. Remove the external dependencies

There was a technical challenge on how to remove the identified external dependencies. Unlike the backend ecosystem where the dependency injection container is popular, the frontend ecosystem is not. There was no framework for dependency management except for the Angular Framework.

We reviewed two libraries that were likely to be the solution, tsyringe and inversify, but we only needed a structure that could inject dependencies right away, so we pushed back the library to a later date. React-based projects required organic injection into components, and we decided to try injecting dependencies by using Context API.

Action 3. Mocking the external dependencies

Although identified as external dependencies, some were simply handled through mocking.

The window objects and Router modules are representative examples. Obviously, managing with external dependencies could have better guaranteed the purity of the business logic, but we judged that the product code change was large and the actual benefit was as much for the cost required for improvement. If the structure is improved a little in the future, I think we can manage the locked part with dependency and remove it from the domain layer.

The output

• We have set up an independent testing environment.
  • If the test fails, it can be solved only by looking at the inside of the test code.
• An easy-to-understand test code has been created.
  • As there was no implicit input and output, we could clearly see how the product works.
  • Just by looking at the test code, we could see the context of writing a new test code.
• It has become easier to analyze business requirements.
  • When there is a logical conflict between existing features and new requirements, it could be verified through the test code.
  • We're using it as a document that best reflects the actual code that works.

Talk is cheap

I'd like to introduce the network layer as an example. We defined the network layer that exists to invoke the server API as external dependency and managed it with an abstraction called APIClient.

Before, the components used to directly approach the client and call the server API.

// AS-IS
import { brandpayClient } from 'remote/brandpayClient';

function Component() {
  return (
    <Button
      onClick={() => brandpayClient.addAccount(data)}
    >
      Register account
    </Button>
  )
}

We modified this in 3 steps:

1. Abstraction

Abstract what is happening at the network layer by defining it as a 'feature' and represent it as an interface called *-Client

interface BrandpayClientService {
  addAccount: ReturnType<typeof addAccount>;
  // ... other features
}

2. Implementation

Implement the 'messages' defined at the client

class BrandpayClient implements BrandpayClientService {
  addAccount(payload: Payload) {
    return this.post(API_PATH, { body: payload });
  }
}

3. Application

Apply the dependencies container (or provider)

function ApiClientProvider() {
  const brandpayClient = useState(() => {
    return new BrandpayClient(params);
  });

  return (
    <BrandpayClientProvider client={brandpayClient}>
      {children}
    </BrandpayClientProvider>
  )
}
function App() {
  return (
    <ApiClientProvider>
      <Component {...props} />
    </ApiClientProvider>
  )
}

Reference the dependencies

function Component() {
  const brandpayClient = useBrandpayClient();

  return (
    <Button
      onClick={() => brandpayClient.addAccount(data)}
    >
      계좌 등록하기
    </Button>
  )
}

4. Test code

it('Should go to completed page after adding account', async () => {
  const user = userEvent.setup();

  render(
    <BrandpayClientProvider
      client={{
        addAccount: () => Promise.resolve(응답_스키마_fixture), 
      }}
    >
      <ARSVerificationPage />
    </BrandpayClientProvider>
  )

  await user.click(await screen.findByText(`인증 전화 받기`));

  await waitFor(() => {
    expect(mockRouter.pathname).toBe(계좌_등록_완료_페이지);
  })
})

What to do next

Runtime error

Now we can trust the test code and proceed with refactoring. Even if we modularize the code and modify the folder structure freely, we can now deploy it with confidence that the test case will pass. However, bugs that occur at runtime are difficult to know at the unit test and integrated test level. To compensate for this, we're considering introducing E2E tests. We're thinking about how to do well because it's an E2E test with a clear trade-off.

Provider hell

In JavaScript, we come across the term 'callback hell'. In our project, we needed multiple layers of providers to inject multiple dependencies when writing test codes. It's a trade-off decision, but it's a lot of work. We regreted that we could have tried to improve the dependencies container better.

// 3개의 중첩 Provider
it('동적으로 약관을 서버에서 불러와 렌더링 한다', async () => {
    render(
      <SDKBridgeTestProvider bridge={{ ... }}>
        <BasicClientProvider client={{ ... }}>
          <AuthClientProvider client={{ ... }}>
            <AgreementPage />
          </AuthClientProvider>
        </BasicClientProvider>
      </SDKBridgeTestProvider>
    );

    expect(await screen.findByText(termTitle)).toBeInTheDocument();
});

The more dependencies you have to manage, the more complexity you have in your management system, such as cooperation between dependencies and dependencies that you have to manage in singleton.

Conclusion

I thought, "The only thing I can trust is the test code." Compared to easily changing reps, the product is always running on the spot. Compared to documents that can't keep up with product changes, the test code could explain how this code works now. The longer the product's lifespan, the more important the test code became.

How can I write the test code well? Knowledge of the test code will be important, and there should be no psychological burden. Furthermore, some said that the habit of writing the test code first is also important.

I also thought, "What if I had designed the project well from the beginning?" We also talked about how if we had an easy design to write a test code, wouldn't it be possible to write it quickly when we need it later even if we don't write it from the beginning.

I thought a lot about what an appropriate designing would be. I think good engineering is the act of finding the most efficient solution to a problem, such as cost, time, fraud, or opportunity. Some costs must be paid immediately and some costs must be borne by technical debt. To do this well, it is important to clearly define and allocate costs appropriately. I hope this article provides an opportunity to judge the cost of the product and reflect on whether it is time to improve it.