🧹

シンプルなCLIをCleanArchitectureとATDDでリファクタリングしていたら、それぞれの恩恵を改めて実感した話

2023/07/26に公開

https://zenn.dev/ryo034/articles/simple-url-open-and-share-cli

こちらの記事の後日譚になります。

リファクタリングの目的と背景

このリファクタリングに至った背景は、初めてCLIを開発した際にいくつか遭遇した課題がきっかけです。

今回のCLI開発ではWeb開発やAPI開発で日常的に利用しているCleanArchitectureとATDDを用いず、手続型のコードと単体テストによってプロジェクトを完成させました。しかし、これは品質に関する観点が十分に考慮されていない状態でした。

CLIを作成するという当初の目的は達成されましたが、その品質については二の次になっておりテストを拡充させる必要がありました。

テストの障壁と認識の変化

CLIのコードをさらに強化し品質を向上させるために、テストを導入しようとした際にいくつかの障壁に直面しました。

  • コマンドオプションの増加による複雑さ
  • ユーザー入力のMock化
  • そしてファイルの読み書きのテスト

これらの問題を解決するためには、単純な手続型のコードでは不十分で、実際にコマンドの入力を行うようなE2Eの導入が必要だと痛感しました。

以下のコードは初期段階に書かれたcmd/list.goのコードになります。上から順にコマンドを受け取り、ファイルからデータを取得し、表示するように手続型で書かれています。cobraのgeneratorで生成されたコードに少しだけ手を加えた程度になっています。

CleanArchitectureとATDDの導入

リファクタリングをするにあたって普段から使用しているCleanArchitectureとATDDで進めることにしました。
この図はCleanArchitectureのイメージ図になります。

cli-clean-architecture-flow

実装

まず上記で記載した手続型のコードではDIすることができません。DIができなければMockを簡単に行うことができません。CleanArchitectureでやる場合にもDIが必要になってくるので、先にDIの基盤を作成します。

まず、空のControllerを作成しそれをDIします

core/interface/controller.go
package controller

type Controller struct {
}

func NewController() *Controller {
	return &Controller{}
}

func (c *Controller) List() error {
	return nil
}

Controllerを注入するためにInjectorを作成します。

core/infrastructure/injector/injector.go
package injector

import (
	"urlo/core/interface/controller"
)

type Injector interface {
	Controller() *controller.Controller
}

type injector struct {
	controller *controller.Controller
}

func NewInjector() Injector {
	return &injector{controller.NewController()}
}

func (i *injector) Controller() *controller.Controller {
	return i.controller
}

これでControllerをInjectする準備ができました。
実際にListでInjectorを読み込んでみたいのですが、今のままだとinjectorを引数に渡して読み込むことができません。

そのためInjectorを読み込めるように以下のようにcmd/list.goコードを修正します.

cmd
package cmd

import (
	"github.com/spf13/cobra"
	"urlo/core/infrastructure/injector"
)

func newListCmd(i injector.Injector) *cobra.Command {
	listCmd := &cobra.Command{
		Use:   "list",
		Short: "List all URLs from the json",
		Run: func(cmd *cobra.Command, args []string) {
			// 元の内容
		},
	}
	listCmd.Flags().BoolP("json", "j", false, "Output in JSON format")
	listCmd.Flags().BoolP("string", "s", false, "Output in JSON String format")
	return listCmd
}

cmd/root.goで以下のように読み込むようにします

cmd/root.go
package cmd

import (
	"github.com/spf13/cobra"
	"os"
	"urlo/core/infrastructure/injector"
)

var rootCmd = &cobra.Command{
	Use:   "urlo",
	Short: "A simple CLI for opening URLs from a json file",
	Long:  "A simple CLI for opening URLs from a json file",
}

func Execute() {
	container := injector.NewInjector()
	rootCmd.AddCommand(newListCmd(container))
	if err := rootCmd.Execute(); err != nil {
		os.Exit(1)
	}
}

これでInjectする準備が整いました。早速テストを書いていきます。

手順としては、以下の手順を繰り返します

  1. 失敗するテストコードを作成
  2. テストを実行し失敗することを確認する
  3. 実装
  4. テストを実行し通ることを確認する

まずシンプルなシナリオ(ファイルに何も書き込まれていない場合に標準出力で'Error: No URL found'と表示される)を一つ想定し失敗するテストファイルを作成します。

cmd/list.go
package cmd

import (
	"github.com/stretchr/testify/assert"
	"testing"
	"urlo/core/infrastructure/injector"
	"urlo/util"
)

func Test_List_Cmd_Output_OK(t *testing.T) {
	tests := []struct {
		name string
		args [][]string
		// -j option
		json string
		// -s option
		jsonString string
		// ターミナルに出力される文字列
		output string
	}{
		{
			name:       "ファイルに何も書き込まれていない場合",
			args:       [][]string{},
			json:       "false",
			jsonString: "false",
			output:     "Error: No URL found\n",
		},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			var err error
			inj := injector.NewInjector()
			listCmd := newListCmd(inj)
			_ = listCmd.Flags().Set("json", tt.json)
			_ = listCmd.Flags().Set("string", tt.jsonString)

			for _, arg := range tt.args {
				listCmd.SetArgs([]string{arg[0], arg[1]})
			}
			fnc := listCmd.Execute
			output, err := util.ExtractStdout(t, fnc)
			assert.NoError(t, err)
			if util.RemoveEscape(output) != tt.output {
				t.Errorf("failed to test. got: %q, want: %q", output, tt.output)
			}
		})
	}
}

Driver

次にDriverから順に作成していきます。

urloではファイルを作成しそれを読み書きすることでURLを管理しています。またユーザーにターミナル上でy/Nの確認をとって処理を続行するような処理もあります。なのでurloでは具体的には以下のものをDriverに書いています。

  • ファイルの作成・参照・書き込み
  • コマンドの実行(URLを開く)
  • プロンプトの実行(ターミナル上でy/Nの確認をとって処理を続行するような処理)

listコマンドにおいてはファイルの作成・参照・書き込みのみなので、ここではその箇所のみを実装します

core/driver/driver.go
package driver

import (
	"encoding/json"
	"fmt"
	"io"
	"os"
	"urlo/core/domain"
)

type Driver interface {
	ReadRecords() ([]domain.UrlMapJson, error)
	Write(v domain.UrlMap) error
	WriteAll(vs *domain.UrlMaps) error
}

type driver struct {
	filePath       string
}

func NewDriver(filePath string) Driver {
	return &driver{filePath}
}

func (d *driver) getOrCreateFile() (*os.File, error) {
	_, err := os.Stat(d.filePath)
	if os.IsNotExist(err) {
		f, err := os.Create(d.filePath)
		if err != nil {
			return nil, fmt.Errorf("create error: %v", err)
		}
		return f, nil
	}
	f, err := os.OpenFile(d.filePath, os.O_RDWR|os.O_CREATE, 0666)
	if err != nil {
		return nil, fmt.Errorf("open error: %v", err)
	}
	return f, nil
}

func (d *driver) ReadRecords() ([]domain.UrlMapJson, error) {
	var urlMaps []domain.UrlMapJson
	f, err := d.getOrCreateFile()
	if err != nil {
		return nil, fmt.Errorf("get file error: %v", err)
	}
	defer func() {
		if closeErr := f.Close(); closeErr != nil {
			if err = fmt.Errorf("defer close error: %v", closeErr); err != nil {
				fmt.Println(err)
			}
		}
	}()

	if err := json.NewDecoder(f).Decode(&urlMaps); err != nil {
		// If the file is empty, return empty
		if err == io.EOF {
			return nil, nil
		}
		return nil, fmt.Errorf("json decode error: %v", err)
	}
	return urlMaps, nil
}

func (d *driver) Write(v domain.UrlMap) error {
	vs, _ := domain.NewUrlMaps([]domain.UrlMap{v})
	return d.WriteAll(vs)
}

func (d *driver) WriteAll(vs *domain.UrlMaps) error {
	f, err := d.getOrCreateFile()
	if err != nil {
		return fmt.Errorf("get file error: %v", err)
	}
	defer func() {
		if closeErr := f.Close(); closeErr != nil {
			if err = fmt.Errorf("defer close error: %v", closeErr); err != nil {
				fmt.Println(err)
			}
		}
	}()

	urlMapJsons := make([]domain.UrlMapJson, 0, vs.Size())
	for _, item := range vs.Values() {
		urlMapJsons = append(urlMapJsons, domain.UrlMapJson{Title: item.Title().String(), URL: item.URL().String()})
	}
	b, err := json.Marshal(urlMapJsons)
	if err != nil {
		return fmt.Errorf("json marshal error: %v", err)
	}
	if _, err = f.Write(b); err != nil {
		return fmt.Errorf("write error: %v", err)
	}
	return nil
}

後のテストのためにNewDriverfilePathを受け取っています。

最初の手続型のコードでテストを書いていただけるとわかりやすいのですが、テストを作成する際にfilePathを受け取らないと、テストでのファイル操作がとても難しくなります。

test.go
inj := injector.NewInjector("test.json")

Gateway

gatewayではdriverを呼び出してdomainに変換します。

core/interface/gateway.go
package gateway

import (
	"urlo/core/domain"
	"urlo/core/driver"
)

type gateway struct {
	driver  driver.Driver
	adapter GatewayAdapter
}

func NewGateway(driver driver.Driver, adapter GatewayAdapter) domain.Repository {
	return &gateway{driver, adapter}
}

func (g *gateway) FindAll() (*domain.UrlMaps, error) {
	rs, err := g.driver.ReadRecords()
	if err != nil {
		return nil, err
	}
	return g.adapter.AdaptAll(rs)
}

Interactor

interactorではrepositoryをビジネスロジック(ここではURLのリストを取得し、画面に表示する)を実装します。
画面に表示する箇所はpresenterが担っています。

core/usecase/interactor.go
package usecase

import (
	"urlo/core/domain"
)

type interactor struct {
	repo      domain.Repository
	presenter OutputPort
}

func NewInteractor(repo domain.Repository, presenter OutputPort) InputPort {
	return &interactor{repo, presenter}
}

func (i *interactor) List(jsonOutput bool, jsonStringOutput bool) error {
	res, err := i.repo.FindAll()
	if err != nil {
		return err
	}
	if res.IsEmpty() {
		i.presenter.NotURLFound()
		return nil
	}

	i.presenter.List(res)
	return nil
}

Controller

上記で作成したcontrollerを改修します。
controllerでは入力を検査しUseCaseが使いやすいように整形してUseCaseを呼び出します。

core/interface/controller.go
package controller

import (
	"fmt"
	"github.com/fatih/color"
	"urlo/core/usecase"
)

type Controller struct {
	interactor usecase.InputPort
}

func NewController(interactor usecase.InputPort) *Controller {
	return &Controller{interactor}
}

func (c *Controller) List(jsonOutput bool, jsonStringOutput bool) error {
	if err := c.interactor.List(jsonOutput, jsonStringOutput); err != nil {
		return err
	}
	return nil
}

root

最後にトップレベルのコマンドの箇所を修正します

cmd/list.go
...
  Run: func(cmd *cobra.Command, args []string) {
    jsonOutput, err := cmd.Flags().GetBool("json")
    if err != nil {
      color.Red("Error: %s\n", err)
      return
    }
    jsonStringOutput, err := cmd.Flags().GetBool("string")
    if err != nil {
      color.Red("Error: %s\n", err)
      return
    }
    if err := i.Controller().List(jsonOutput, jsonStringOutput); err != nil {
      color.Red("Error: %s\n", err)
      return
    }
    return
  },
  ...

これでlistコマンドの内部の実装が終わりました。
テストを実装すると通るかと思います。

  • ファイルに何も書き込まれていない場合

テストの追加と修正

現状は"ファイルに何も書き込まれていない場合"のテストしかありません。なので以下のテストを追加します。

  • すでにデータがある場合に正しく表示されること
  • -jオプションが設定されている場合にjson形式で出力される
  • -sオプションが設定されている場合にjson文字列形式で出力される
  • -sと-jオプションが両方設定されている場合にエラーが出力される

これらを満たすために以下のようにテストを書き直します

cmd/list_test.go
package cmd

import (
	"encoding/json"
	"fmt"
	"github.com/stretchr/testify/assert"
	"io"
	"os"
	"reflect"
	"testing"
	"urlo/core/domain"
	"urlo/core/infrastructure"
	"urlo/core/infrastructure/injector"
	"urlo/util"
)

func Test_List_Cmd_Output_OK(t *testing.T) {
	tests := []struct {
		name       string
		args       [][]string
		json       string
		jsonString string
		prepare    []domain.UrlMapJson
		want       []domain.UrlMapJson
		output     string
	}{
		{
			name:       "ファイルに何も書き込まれていない場合",
			args:       [][]string{},
			json:       "false",
			jsonString: "false",
			prepare:    nil,
			want:       []domain.UrlMapJson{},
			output:     "Error: No URL found\n",
		},
		{
			name:       "データがすでに存在する場合",
			args:       [][]string{},
			json:       "false",
			jsonString: "false",
			prepare:    []domain.UrlMapJson{{Title: "google", URL: "https://google.com"}},
			want:       []domain.UrlMapJson{{Title: "google", URL: "https://google.com"}},
			output:     "google - https://google.com\n",
		},
		{
			name:       "json形式で出力する場合",
			args:       [][]string{},
			json:       "true",
			jsonString: "false",
			prepare:    []domain.UrlMapJson{{Title: "google", URL: "https://google.com"}},
			want:       []domain.UrlMapJson{{Title: "google", URL: "https://google.com"}},
			output: `[
  {
    "title": "google",
    "url": "https://google.com"
  }
]
`,
		},
		{
			name:       "json文字列形式で出力する場合",
			args:       [][]string{},
			json:       "false",
			jsonString: "true",
			prepare:    []domain.UrlMapJson{{Title: "google", URL: "https://google.com"}},
			want:       []domain.UrlMapJson{{Title: "google", URL: "https://google.com"}},
			output:     "[{\"title\":\"google\",\"url\":\"https://google.com\"}]\n",
		},
		{
			name:       "-jと-sを同時に指定した場合にエラーになる",
			args:       [][]string{},
			json:       "true",
			jsonString: "true",
			prepare:    []domain.UrlMapJson{{Title: "google", URL: "https://google.com"}},
			want:       []domain.UrlMapJson{{Title: "google", URL: "https://google.com"}},
			output:     "Can't use both -j and -s\n",
		},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			var err error
			// テスト終了のタイミングでファイルを自動で削除
			defer func() {
				if err = os.Remove(util.TestFilePath); err != nil {
					fmt.Println("remove file error")
					panic(err)
				}
			}()

			// テスト用のファイルを作成して、テストデータを書き込む
			if tt.prepare != nil {
				f, err := os.Create(util.TestFilePath)
				if err != nil {
					panic(err)
				}
				if err = json.NewEncoder(f).Encode(&tt.prepare); err != nil {
					panic(err)
				}
			}

			inj := injector.NewInjector(util.TestFilePath, infrastructure.NewCommandExecutor(), infrastructure.NewPromptExecutor())
			listCmd := newListCmd(inj)
			_ = listCmd.Flags().Set("json", tt.json)
			_ = listCmd.Flags().Set("string", tt.jsonString)

			// テスト用の引数をセット
			for _, arg := range tt.args {
				listCmd.SetArgs([]string{arg[0], arg[1]})
			}
			fnc := listCmd.Execute
			output, err := util.ExtractStdout(t, fnc)
			assert.NoError(t, err)

			// 出力結果のテスト
			if util.RemoveEscape(output) != tt.output {
				t.Errorf("failed to test. got: %q, want: %q", output, tt.output)
			}
			f, err := os.Open(util.TestFilePath)
			if err != nil {
				t.Errorf("newListCmd() error = %v", err)
			}
			var urlMaps []domain.UrlMapJson
			if err = json.NewDecoder(f).Decode(&urlMaps); err != nil {
				if err == io.EOF {
					return
				}
				t.Errorf("newListCmd() error = %v", err)
				return
			}
			// ファイルの中身のテスト
			if !reflect.DeepEqual(urlMaps, tt.want) {
				t.Errorf("newListCmd() got = %v, want %v", urlMaps, tt.want)
			}
		})
	}
}

テストを修正したらテストを実行します。実行結果は以下になります。

  • ファイルに何も書き込まれていない場合
  • データがすでに存在する場合
  • json形式で出力する場合
  • json文字列形式で出力する場合
  • -jと-sを同時に指定した場合にエラーになる

テストに通ってないシナリオをグリーンにするために実装を修正します。

json形式で出力する場合json文字列形式で出力する場合は出力に関するシナリオなのでinteractorに新たに追記します

core/usecase/interactor.go
func (i *interactor) List(jsonOutput bool, jsonStringOutput bool) error {
	res, err := i.repo.FindAll()
	if err != nil {
		return err
	}
	if res.IsEmpty() {
		i.presenter.NotURLFound()
		return nil
	}

	// ここから追記
	if jsonOutput {
		return i.presenter.ListAsJson(res)
	}

	if jsonStringOutput {
		return i.presenter.ListAsJsonString(res)
	}
	// ここまで追記

	i.presenter.List(res)
	return nil
}

これでテストを実行すると結果は以下のようになります。

  • ファイルに何も書き込まれていない場合
  • データがすでに存在する場合
  • json形式で出力する場合
  • json文字列形式で出力する場合
  • -jと-sを同時に指定した場合にエラーになる

-jと-sを同時に指定した場合にエラーになるはユーザーの入力に関するバリデーションなのでcontrollerに追記します

core/interface/controller.go
func (c *Controller) List(jsonOutput bool, jsonStringOutput bool) error {
	// ここから追記
	if jsonOutput && jsonStringOutput {
		fmt.Println("Can't use both -j and -s")
		return nil
	}
	// ここまで追記
	if err := c.interactor.List(jsonOutput, jsonStringOutput); err != nil {
		return err
	}
	return nil
}

これでテストを実行すると結果は以下のようになります。

  • ファイルに何も書き込まれていない場合
  • データがすでに存在する場合
  • json形式で出力する場合
  • json文字列形式で出力する場合
  • -jと-sを同時に指定した場合にエラーになる

無事全てのテストが通るようになりました🎉🎉🎉🎉


これで実装は以上になります。この実装の中で不明点はいくつかあると思いますが、気になる方はリポジトリを確認してみてください

https://github.com/ryo034/homebrew-urlo

ATDDとを導入することで、明確に定義されたユーザーの入力に対するテストが簡単になりました。
また、CleanArchitectureを用いることでファイルの読み書きをMock化することが容易になり、テストをより簡単に行うことができました。

CleanArchitectureとATDD導入後の影響

今回の経験を通じて、CLI開発にCleanArchitectureとATDDを導入することで、テストの効率が大幅に向上し、コードの品質が確保できることを確認しました。
また、コマンドオプションが増えた場合や、ユーザー入力やファイル操作が複雑化した場合でも、Mockを用いて簡単にテストを書くことができるため、開発速度を保ちつつ品質も確保できるという利点を実感しました。

まとめ

今回のCLIのようなシンプルな実装にCleanArchitectureやATDDを用いることは過剰だと思われる方もいると思いますが、実際に実装していくと、最初から導入することで品質と開発速度を保ち安全にリリースすることができると実感しました。
また、これらの手法は、Web開発やAPI開発だけでなくCLI開発にも十分に応用可能で、コードの品質と開発の効率を高める重要なツールであると改めて認識しました。

手続型からCleanArchitectureやATDDに移行する方法がわからない方や、その具体的な手法について不明な点がある方がいたら、私の経験が少しでも参考になれば幸いです。

Discussion