gitlab-org--gitlab-foss/doc/development/refactoring_guide/index.md

# Refactoring guide

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 (e.g. anything that's injected into the template or used in a conditional).
1. For each possible input, identify the significant possible values.
1. 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.
1. Run all the pinning tests against the code, before you start refactoring (Oracle)
1. Perform the refactor (or checkout the commit with the work done)
1. Run again all the pinning test against the post refactor code (Pin)
1. Compare the Oracle with the Pin. If the Pin is different, you know the refactoring doesn't preserve existing behavior.
1. 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 checkout 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](https://gitlab.com/gitlab-org/gitlab/-/commit/7b73da4078a60cf18f5c10c712c66c302174f506?merge_request_iid=29528#a061e6835fd577ccf6802c8a476f4e9d47466d16_0_23):

```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](http://wiki.c2.com/?PinningTests)

### Examples

- [Pinning test in a Haml to Vue refactor](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/27691#pinning-tests)
- [Pinning test in isolating a bug](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/32198#note_212736225)
- [Pinning test in refactoring dropdown](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/28173)
- [Pinning test in refactoring vulnerability_details.vue](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25830/commits)
- [Pinning test in refactoring notes_award_list.vue](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29528#pinning-test)
- [Video of pair programming session on pinning tests](https://youtu.be/LrakPcspBK4)