テスト書いてないのを見つけたら、テスト書こうよ

これはHubble Advent Calendar 2023の15日目の記事です。

はじめに

こんにちは！株式会社Hubbleでバックエンドエンジニアをしている、@ktkr-wksです。

ところでみなさんテスト書いてますか？テスト書いてますよね？？？
Hubbleではもちろんテスト書いてますが、シード立ち上げ時期に勢いで書いていた時代のコードで且つ数少ないJavaで書かれている箇所にはテストが書かれていないものが残っていました。

過去ではJavaは進化が遅い言語というイメージがありましたが、今では言語仕様の変更が頻繁に行われ、Hubbleで利用しているSpringBootもマイナーバージョンのEOLは1年でやってきます。
常にメンテナンスを行う必要があるということは、テストを書かないと品質を担保できなくなりました。

そこで今回は、テストを書かれていなかったサービスにテストを書いたり、リファクタリングの過程を紹介します。

この@RestController、一貫性なさすぎ

先ず、テストを書く箇所はエンドポイントから手をつけました。
当時このバックエンドサービスの@RestControllerの各メソッドには@ModelAttributeと@RequestParamとその両方を利用したものが入り乱れていました。
先ずはテストを書いて、考え方はいろいろあると思いますが今回は@ModelAttributeに寄せた書き方へ直しました。
テストから書くのが重要で、修正前と修正後で同じ動きをここで担保しています。

修正前

@RestController
class FugaController {
    @RequestMapping("/hoge")
    public void getHoge(@ModelAttribute CommonParams param, @RequestParam("foo") String foo, @RequestParam("bar") String bar) {
        var hoge = new Hoge(param.getAaa(), param.getBbb(), foo, bar);

追加したテスト

@WebMvcTest(FugaController.class)
public class FugaControllerTest {
	@Autowired
	private MockMvc mockMvc;

	@Test
	public void testGetHoge_Success() {
		mockMvc.perform(get("/hoge?aaa=aaa&bbb=aaa&foo=foo&bar=bar"))
				.andExpect(status().isOk());
	}

修正後

1つのModelに詰める様に修正

@RestController
class FugaRestController {
    @RequestMapping("/hoge")
    public void getHoge(@ModelAttribute HogeModel model) {
        var hoge = new Hoge(model.getAaa(), model.getBbb(), model.getFoo(), model.getBar());

でもこの@RestController、そのままテスト書けない

当時の各メソッドにはnewが毎回出てきていました。またそのクラスは処理が複雑なものが実装されていました。これはコントロールのテストするならばMockにしたいですがこのままだとテスト出来ません。
新規にそのクラスをnewするためだけの@Serviceクラスを作成し、インターフェースも用意して疎結合にしました。

修正後

@Serviceを作成し、それを利用

@Service
class HogeServiceImpl implements HogeService {
    public Hoge create(final String aaa, final String bbb, final String foo, final String bar) {
        return new HogeImpl(aaa, bbb, foo, bar);
    }
}

@RestController
@RequiredArgsConstructor
class FugaRestController {
    private final HogeService hogeService;
    
    @RequestMapping("/hoge")
    public void getHoge(@ModelAttribute HogeModel model) {
        var hoge = HogeService.create(model.getAaa(), model.getBbb(), model.getFoo(), model.getBar());

修正したテスト

@WebMvcTest(FugaController.class)
public class FugaControllerTest {
    @Autowired
    private MockMvc mockMvc;

    @MockBean
    private HogeService hogeService;

    @Mock
    private Hoge hoge;

    @BeforeEach
    public void setUp() throws Exception {
        when(hogeService.create(anyString(), anyString(), anyString(), anyString())).thenReturn(hoge);
    }

    @Test
    public void testGetHoge_Success() {

でもでもこの@RestController、バリデーションない

当バックエンドサービスではクライアント側からのアクセスはなく、他のバックエンドから呼ばれるAPIしかありません。
ですが、考え方としてはサービスを跨ぐ際は必ずバリデーションを用意しておかないと、呼び出し側が不意の修正でバグの発見が遅れる可能性があります。
バリデーションの用意は重要です。

修正後

省略してますが、HogeModelには各パラメータに対するバリデーション処理を記したアノテーションを付与

    @RequestMapping("/hoge")
    public void getHoge(@ModelAttribute @Validated HogeModel model) {

例外処理を集中管理するクラスの作成。この実装がないと、そのままスタックトレースが出力されてしまいます。

@ControllerAdvice
public class ValidatedExceptionHandler {
    @ExceptionHandler(MethodArgumentNotValidException.class)
    public ResponseEntity<?> handleMethodArgumentNotValid() {
        return ResponseEntity.badRequest().body(new ErrorResponse(HttpStatus.BAD_REQUEST.value(), "Invalid request"));
    }
}

修正したテスト

@WebMvcTest(FugaController.class)
@ContextConfiguration
public class FugaControllerTest {
    @Autowired
    private WebApplicationContext context;

    private MockMvc mockMvc;

    @MockBean
    private HogeService hogeService;

    @Mock
    private Hoge hoge;

    @BeforeEach
    public void setUp() throws Exception {
        mockMvc = MockMvcBuilders.webAppContextSetup(context).build();
        when(hogeService.create(anyString(), anyString(), anyString(), anyString())).thenReturn(hoge);
    }

    @Test
    /* 略 */
    @Test
    public void testGetHoge_Valid() throws Exception {
        mockMvc.perform(get("/hoge?aaa=&bbb=aaa&foo=foo&bar=bar"))
                .andExpect(status().isBadRequest())
                .andExpect(jsonPath("$.message").value("Invalid request"));
        /* 続く */

まとめ

どの環境どの言語でもそうですが、テストを書き始めなければそのうち品質はどんどん落ちていきます。
今回の紹介はあくまでもRestController内でしたが、実際は内部の処理も同様の切り分けでテストを書いていってます。
先ず今あるコードをフレッシュに保つためにもテストを書いてみませんか？

明日は@power3812さんです。