Goで時刻が絡む単体テストをどうするか考える

Goにはtimeパッケージをmockする仕組みが用意されていない。
そのため、time.Nowを以下のようにして使っているコードを見かけることがある。

var Now = func() time.Time { return time.Now() }

たしかにこうすることでNow()が返す値を自由に設定でき、単体テストが書きやすくなる。
ただ、関数を変数に入れるということは「その関数を処理の中で書き換えるため」という意味を持たせることになり、それをテストでしか書き換えないのであれば読み手に無用な混乱を与えてしまいそうで個人的にはあまり好きではない。

その辺りの意識が開発者の間で統一されていればそれで良いのかもしれないが、新しく入った人なんかはそうもいかないはず。
さすがに意図せず書き換えてトラブってしまうことはまず無いだろうが、自分がよく使いそうな用途でこのような使い方を避けられないか考えてみようと思う。

時刻を含むデータが返ってくる処理

例えばタイムスタンプを付与したリソースを作成するような関数。

type Resource struct {
    Data       string
    CreateTime time.Time
}

func NewResource(data string) *Resource {
    return &Resource{
        Data:       data,
        CreateTime: time.Now(),
    }
}

当然NewResource()のテストでreflect.DeepEqualを使うと通らない。

func TestNewResource(t *testing.T) {
    want := &Resource{Data: "test", CreateTime: time.Now()}

    got := NewResource("test")
    if !reflect.DeepEqual(got, want) {
        // got.CreateTimeとwant.CreateTimeが一致しない
        t.Errorf("NewResource() = %v, want %v", got, want)
    }
}

ただ、ここでCreateTimeの値を確認したいかというと必ずしもそうではない。
なので不要なフィールドは除外して比較する、で十分なケースがほとんどなはず。

func resetFields(r *Resource) *Resource {
    // 元の値は変えず、CreateTimeだけゼロ値にしたコピーを作る
    after := *r
    after.CreateTime = time.Time{}
    return &after
}

func TestNewResource_reset(t *testing.T) {
    want := &Resource{Data: "test"}

    got := NewResource("test")
    after := resetFields(got)
    if !reflect.DeepEqual(after, want) {
        t.Errorf("NewResource() = %v, want %v", after, want)
    }
}

毎回resetFields()のような関数を作るのが面倒であればgo-cmpを使うとcmpopts.IgnoreFieldsに除外するフィールドを設定するだけになるので簡単だ。

func TestNewResource_ignore(t *testing.T) {
    want := &Resource{Data: "test"}
    opt := cmpopts.IgnoreFields(Resource{}, "CreateTime")

    got := NewResource("test")
    if !cmp.Equal(got, want, opt) {
        t.Errorf("NewResource() = %v, want %v", got, want)
    }
}

どうしてもtime.Nowが使われていていることを確認したければ、それはテストではなく静的解析をしてチェックしてあげれば良いんじゃないだろうか。

ということでtime.Nowを書き換える必要は無さそう。

時刻を使用した条件判定がある処理

例えば以下のような関数。

// GetResource は指定されたidのResourceを返す。Resourceの取得に失敗、
// もしくは取得したResourceの有効期限が切れていた場合はエラーを返す。
func (s *service) GetResource(id int64) (*Resource, error) {
    res, err := s.store.Get(id)
    if err != nil {
        return nil, err
    }

    if time.Now().After(res.ExpireTime) {
        return nil, ErrExpired
    }

    return res, nil
}

こういったものは時刻使った判定処理を関数やメソッドに切り出し、その部分を単体でテストできるようにしてしまう。

func IsExpired(r *Resource, t time.Time) bool {
    return t.After(r.ExpireTime)
}

// or

func (r *Resource) IsExpired(t time.Time) bool {
    return t.After(r.ExpireTime)
}

正直GetResource()がエラーを返す場合のテストケースを網羅するよりも、呼び出し元がそのエラーハンドリングを正しく実装できているかを確認する方がよっぽど大事だと思っている。
呼び出し元は次のようなREST APIのハンドラだったりするのでserviceはモックなどにしてテストすることになり、そうなるとtime.Nowを書き換える必要は無くなる。

func (h handler) Get(w http.ResponseWriter, r *http.Request) {
    id, err := getIDFromPath(r.URL.Path)
    if err != nil {
        http.NotFound(w, r)
        return
    }

    res, err := h.service.GetResource(id)
    if err != nil {
        if err == ErrExpired {
            http.NotFound(w, r)
        } else {
            http.Error(w, err.Error(), http.StatusInternalServerError)
        }
        return
    }

    json.NewEncoder(w).Encode(res)
}

以前から気にはなっていたけど、そういうパッケージやTesting in Go by example: Part 5にあるようなパターンを見てもあまりピンと来ず、
色々な人に聞いてみても「time.Nowを変数に入れるのは仕方ない」という意見ばかりだったのでちょっと考えてみた。

  • 「引数で渡そうとしても結局はどこかでtime.Nowを呼ばなきゃいけなくなる」
    • time.Nowを書き換える必要がなければ良いのでは?
  • 「引数にtime.Timeを渡したくない」
    • 好みの問題ではあるけど、そこまで不自然じゃなければ良いのでは?
    • 渡されたtを基に○○する関数 なんかはそんなに違和感なさそう
  • 「都合によりどうしてもテストしなければならないが、他に良い方法が無い」
    • そういう場合はたしかに
  • 「そもそも何が悪いのかわからない」
    • ...

複雑なケースになってくるとどうなるかわからないけど、うまいこと設計できればなんとかなりそうな気がするのでどこかで試してみたい。