ContextQMD
Back to Gitlabhq

Refactoring guide

doc/development/refactoring_guide/_index.md

18.11.23.5 KB
Original Source

This document is a collection of techniques and best practices to consider while performing a refactor.

Pinning tests

Pinning tests help you ensure that you don't unintentionally change the output or behavior of the entity you're refactoring. This even includes preserving any existing buggy behavior, since consumers may rely on those bugs implicitly.

Example steps

  1. Identify all the possible inputs to the refactor subject (for example, anything that's injected into the template or used in a conditional).
  2. For each possible input, identify the significant possible values.
  3. Create a test to save a full detailed snapshot for each helpful combination values per input. This should guarantee that we have "pinned down" the current behavior. The snapshot could be literally a screenshot, a dump of HTML, or even an ordered list of debugging statements.
  4. Run all the pinning tests against the code, before you start refactoring (Oracle)
  5. Perform the refactor (or check out the commit with the work done)
  6. Run again all the pinning test against the post refactor code (Pin)
  7. Compare the Oracle with the Pin. If the Pin is different, you know the refactoring doesn't preserve existing behavior.
  8. Repeat the previous three steps as necessary until the refactoring is complete.

Example commit history

Leaving in the commits for adding and removing pins helps others check out and verify the result of the test.

shell
AAAAAA Add pinning tests to funky_foo
BBBBBB Refactor funky_foo into nice_foo
CCCCCC Remove pinning tests for funky_foo

Then you can leave a reviewer instructions on how to run the pinning test in your MR. Example:

First revert the commit which removes the pin.

shell
git revert --no-commit $(git log -1 --grep="Remove pinning test for funky_foo" --pretty=format:"%H")

Then run the test

shell
yarn run jest path/to/funky_foo_pin_spec.js

Try to keep pins green

It's hard for a refactor to be 100% pure. This means that a pin which captures absolutely everything is bound to fail with some trivial and expected differences. Try to keep the pins green by cleaning the pin with the expected changes. This helps others quickly verify that a refactor was safe.

Example:

javascript
// funky_foo_pin_spec.js

const cleanForSnapshot = el => {
  Array.from(rootEl.querySelectorAll('[data-deprecated-attribute]')).forEach(el => {
    el.removeAttribute('data-deprecated-attribute');
  });
};

// ...

expect(cleanForSnapshot(wrapper.element)).toMatchSnapshot();

Resources

Unofficial wiki explanation

Examples