経験は何よりも饒舌

10年後に真価を発揮するかもしれないブログ 

DefinitelyTyped で Ace の型を大幅に改善した


前のこの記事では、Definitely Typedから型をインストールして適用する際に、実際にレポジトリまで見に行かないといけなかった、ということを書いた。

wafuwafu13.hatenadiary.com

今回は、PRを出してマージされたので、そのことを書く。

github.com

PRタイトルにある通り、この型は全て ajaxorg/ace/ace.d.ts から取ってきた。
コミットログを見ると、ace.d.tsファーストコミットが2018年3月DefinitelyTyped/types/ace/index.d.tsファーストコミットが2017年3月 となっており、DefinitelyTypedの方が先に作成されているが、以下のコメントに示す通り、

[ace] Improve by referring to ace.d.ts by wafuwafu13 · Pull Request #51346 · DefinitelyTyped/DefinitelyTyped · GitHub

ace.d.tsの型の方が正確であり、以下の差分に示す通り、

[ace] Improve by referring to ace.d.ts by wafuwafu13 · Pull Request #51346 · DefinitelyTyped/DefinitelyTyped · GitHub

DefinitelyTypedの型にはanyが多く、その役割をきちんと果たしてはいなかった。
DefinitelyTypedのレポジトリは巨大で、cloneするにも検索するにも時間がかかって大変だったが、PRを出した。
ace.d.tsを参考にしてるとはいえ、変更がかなり多いので、レビュワーが大変だなーと思っていたが、2週間経ってもレビュワーは反応しなかった。
botが反応し、違う人にレビュー依頼を出してからは、すぐにマージされてリリースされた。
[ace] Improve by referring to ace.d.ts by wafuwafu13 · Pull Request #51346 · DefinitelyTyped/DefinitelyTyped · GitHub

anyだらけで長い間放置されていたことも、なかなかレビューがされなかったことも、違う人にレビューが回ると一瞬でマージされたことにも、DefinitelyTypedに対する今までの信頼性(主観)が少し損なわれたが、改善できたのはよかった。

参考にしていた、ace.d.tsにも、まだ足りない型定義があったので少し追加しておいた。
github.com

こちらの反応は早かった。

Jest のモック関数を整理する


npqという、npmやyarnでインストールする際に安全確認をするライブラリがあり、そこで使われているaxiosnode-fetchで代替するというissueがあったのでやってみた。

github.com
github.com

置き換えるだけの簡単な作業かと思っていたが、テストを通すのが難しかった。
jest.mockmockImplementationjest.spyOnに対する理解が曖昧だったので、整理したいと思う。
コードはこのレポジトリにまとめた。
github.com


まずは、単純な関数のモックから整理していきたい。
下のように、第2引数とランダムな値を足した値を返す関数があるとする。

function random(a, b, c) {
  return b + Math.random();
}

module.exports = random;

この関数のテストでは、便宜上、ランダムな値は無視したい。
なぜなら、無視しないと返り値が予測できず、テストが通らないからだ。

const random = require("./random");

it("random function", () => {
  expect(random(1, 2, 3)).toBe(2);
  // Expected: 2
  // Received: 2.119340184508286
});

モックする1つの方法として、jest.fn を用い、randomに新しい、未使用の mock functionを代入してしまう方法がある。
しかし、この方法では、後続のテストが壊れてしまう恐れがある。

let random = require("./random");

random = jest.fn(() => 2)

it("random function", () => {
  expect(random(1, 2, 3)).toBe(2);
  // pass
  expect(random(1, 3, 5)).toBe(3);
  // Expected: 3
  // Received: 2
});

そこで、randomモジュールをモックし、random関数が第2引数を返すようにモックすれば、うまくいく。

const random = require("./random");

jest.mock('./random')
random.mockImplementation((a, b, c) => b);

it("random function", () => {
  expect(random(1, 2, 3)).toBe(2);
  // pass
  expect(random(1, 3, 5)).toBe(3);
  // pass
});

これをnode-fetchに対するモックでも応用してみる。
下のように、Promiseを返す関数があるとする。

const fetch = require("node-fetch");

function getByFetch() {
  const data = fetch("http://example.com/");
  return data;
}

module.exports = getByFetch;

これに対するテストは、node-fetchモジュールをモックし、fetch関数がfooを返すようにモックすれば、うまくいく。

const fetch = require("node-fetch");
const getByFetch = require("./fetch");

jest.mock("node-fetch");
fetch.mockImplementation(() => {
  return "foo";
});

it("fetch", () => {
  const data = getByFetch();
  expect(data).toBe("foo");
  // pass
});

次に、オブジェクト内の関数のモックについて整理していく。
このようなオブジェクトがあるとする。

const obj = {
  random: function () {
    return Math.random();
  },
  randomPlusOne: function () {
    return Math.random() + 1;
  },
   foo: function () {
    return "foo";
  },
};

module.exports = obj;

これに対するテストでモックする1つの方法としては、objモジュールごとモックしてしまう方法がある。

const obj = require("./object");

jest.mock("./object", () => {
  return {
    random: () => 1,
    randomPlusOne: () => 2,
    foo: () => 'foo',
  };
});

it("random object", () => {
  expect(obj.random()).toBe(1);
  // pass
  expect(obj.randomPlusOne()).toBe(2);
  // pass
  expect(obj.foo()).toBe('foo')
  // pass
});

これは、次のようにも書ける。

const obj = require("./object");

jest.mock("./object");
obj.random.mockImplementation(() => 1);
obj.randomPlusOne.mockImplementation(() => 2);
obj.foo.mockImplementation(() => 'foo');

it("random object", () => {
  expect(obj.random()).toBe(1);
  // pass
  expect(obj.randomPlusOne()).toBe(2);
  // pass
  expect(obj.foo()).toBe('foo')
  // pass
});

しかしこれらは、objモジュールごとモックしているため、例えばfooをモックし忘れたら、存在していないことになってしまう。

const obj = require("./object");

jest.mock("./object");
obj.random.mockImplementation(() => 1);
obj.randomPlusOne.mockImplementation(() => 2);

it("random object", () => {
  expect(obj.random()).toBe(1);
  // pass
  expect(obj.randomPlusOne()).toBe(2);
  // pass
  expect(obj.foo()).toBe('foo')
  //  Expected: "foo"
  //  Received: undefined
});

jest.spyOnを用いれば、その心配はなくなる。

const obj = require("./object");

jest.spyOn(obj, 'random').mockImplementation(() => 1);
jest.spyOn(obj, 'randomPlusOne').mockImplementation(() => 2);

it("random object", () => {
  expect(obj.random()).toBe(1);
  // pass
  expect(obj.randomPlusOne()).toBe(2);
  // pass
  expect(obj.foo()).toBe('foo')
  // pass
});

これをaxiosに対するモックでも応用してみる。
下のように、Promiseを返す関数があるとする。

const axios = require("axios");

function getByAxios() {
  const data = axios.get("http://example.com/");
  return data;
}

module.exports = getByAxios;

これに対するテストは、以下のように書けばうまくいく。

const axios = require("axios");
const getByAxios = require("./axios");

jest.spyOn(axios, 'get').mockImplementation(() => Promise.resolve('foo'));

it("axios", async () => {
  const data = await getByAxios();
  expect(data).toBe("foo");
});

理解が少し難しいjest.mock/mockImplementation/jest.spyOn あたりを整理した。

Go の Short variable declarations と Named return values


go-mp4という、mp4ファイルをパースしてくれるGoで書かれたライブラリがあった。
Goに慣れるため、golintカバレッジを上げるPRを出してみた。

github.com

自分の書いたコードで、:=ではno new variables on left side of :=というエラーが出たけれど、代わりに=を使えばうまくいく、という事象があった。
具体的には、以下の部分でエラーが出ていた。

func UnmarshalAny(r io.ReadSeeker, boxType BoxType, payloadSize uint64, ctx Context) (box IBox, n uint64, err error) {
	dst, err := boxType.New(ctx)
	if err != nil {
		return nil, 0, err
	}
	n, err := Unmarshal(r, payloadSize, dst, ctx) // no new variables on left side of :=
	return dst, n, err
}

Goの言語仕様のShort_variable_declarationsによると、同じブロック内で同じ型として宣言された変数に対して、宣言する変数のうち少なくともひとつはブランク変数でなければ、再宣言が可能ということだった。

Unlike regular variable declarations, a short variable declaration may redeclare variables provided they were originally declared earlier in the same block (or the parameter lists if the block is the function body) with the same type, and at least one of the non-blank variables is new.

ただし今回は、dst, err := boxType.New(ctx)に対する再宣言が原因でエラーが出ているわけではない。
以下のコードがコンパイルエラーを起こさないことがそれを証明している。
(本来dstIBoxという型だが、ここではstringにしている。)

func main() {
	dst, err := "foo", fmt.Errorf("error1")
	n, err := 1, fmt.Errorf("error2")
	
	fmt.Println(dst,n,err) // foo 1 error2
}

試しに以下のように、関数内で使われているnという変数名をn1に変えてみると、no new variables on left side of :=というエラーは起こらなかった。

func UnmarshalAny(r io.ReadSeeker, boxType BoxType, payloadSize uint64, ctx Context) (box IBox, n uint64, err error) {
	dst, err := boxType.New(ctx)
	if err != nil {
		return nil, 0, err
	}
        // n1に変えた
	n1, err := Unmarshal(r, payloadSize, dst, ctx)
	return dst, n1, err
}

また、戻り値の変数の名前(named return value)をnからn1に変えてみても、no new variables on left side of :=というエラーは起こらなかった。

func UnmarshalAny(r io.ReadSeeker, boxType BoxType, payloadSize uint64, ctx Context) (box IBox, n1 uint64, err error) { // n1に変えた
	dst, err := boxType.New(ctx)
	if err != nil {
		return nil, 0, err
	}
	n, err := Unmarshal(r, payloadSize, dst, ctx)
	return dst, n, err
}

A Tour of Go の Named return values によると、nerrorは関数の最初で定義した変数名として扱われるということだった。

Goでの戻り値となる変数に名前をつける( named return value )ことができます。戻り値に名前をつけると、関数の最初で定義した変数名として扱われます。
この戻り値の名前は、戻り値の意味を示す名前とすることで、関数のドキュメントとして表現するようにしましょう。

つまり、以下のコードが通らず、

func main() {
        var n uint64
        var err error
	
	n, err := 1, fmt.Errorf("error") // no new variables on left side of :=
}

以下のコードが通る、ということが今回起きていたのだった。

func main() {
        var n uint64
        var err error
	
	n, err = 1, fmt.Errorf("error")
}


Goの言語仕様でnamed return valueの仕様は、さっとみた限りヒットしなかったが、Defer statementsの欄に、

function has named result parameters that are in scope within the literal, the deferred function may access and modify the result parameters before they are returned.

と書いてあった。

エラーを報告する際に意識していること


基本的には質問をせずに粘ってしまうタイプなのだが、環境構築の段階でのエラーや、人生で初見のエラーなどは経験上、粘っても何も得られないので、即質問するようにしている。

そこで意識していることは、

  • 何が原因で何ができなかったのかを記す
  • 自分が打ったコマンドのログを全て記す
  • なぜそのコマンドを打ったのかの情報源を記す
  • その結果のログを記す
  • 自分なりの解決案があれば記す

具体例
github.com

qrcode.react とスナップショットテストの相性が最高すぎた


qrcode.reactは、その名の通りQRコードを生成してくれるReactコンポーネントだ。
github.com

ファイル構成はsrc/index.jsでゴリゴリ計算というか文字列を算出し、コンポーネントを返す構成だった。
テストがなかったので、テストを追加したかったのだが、何に対してテストを追加すればよいのかわからず、しばらく放置していた。
convertStr関数やgeneratePath関数の入力と出力に対する単体テストを追加しようと思ったけれど、あまり恩恵はないように思えた。
qrcode.react/index.js at 6aeb42abc26ffecc868b630b6ad8f507d2125813 · zpao/qrcode.react · GitHub
qrcode.react/index.js at 6aeb42abc26ffecc868b630b6ad8f507d2125813 · zpao/qrcode.react · GitHub


そんなとき、以下の記事を目にした。
www.mizdra.net

試しにqrcode.reactで試してみると、とてもよい結果が得られた。

github.com

何がよかったかと思うと、convertStr関数やgeneratePath関数の入力と出力に対する単体テストをせずとも、それらがうまく機能していることを保証でき、なにより、pathコンポーネントdの値の複雑性がスナップショットの出力により明らかになり、スナップショットの結果が1つの情報としても機能しているところだ。

  <path
    d="M0,0 h29v29H0z"
    fill="#ffffff"
  />
  <path
    d="M0 0h7v1H0zM8 0h1v1H8zM10 0h5v1H10zM17 0h2v1H17zM22,0 h7v1H22zM0 1h1v1H0zM6 1h1v1H6zM9 1h2v1H9zM13 1h1v1H13zM19 ....
    fill="#000000"
  />

マージされなくてもブログを書けば供養できると思って書いた。

npmから@typesを使う場合はDefinitelyTypedを見に行かないといけないことがある

Link Preview JSという、「allows you to extract information from a HTTP url/link (or parse a HTML document) and retrieve meta information such as title, description, images, videos, etc」なレポジトリがあった。

github.com

そこでは、cheerioというライブラリを用いて、パースされたレスポンスから特定の要素を取得している。
https://github.com/ospfranco/link-preview-js/blob/d22888f26718674d7a32cfb935434f7357c80817/index.ts#L236

Link Preview JSはTypeScriptで書かれているのだが、cheerioに対する型が全くなく、anyが使用されていたので、@types/cheerioを導入した。

github.com

だが、一部だけ以下のようなエラーがでて、@types/cheerioを用いてもanyが残ってしまっていた。

Property 'attribs' does not exist on type 'Element'.
Property 'attribs' does not exist on type 'TextElement'. ts(2339)

https://github.com/ospfranco/link-preview-js/pull/83#discussion_r563268635


このまま放置するのも気持ちが悪かったので、実際に@types/cherrioの型が書かれているDefinitely Typedをみにいった。

github.com
DefinitelyTyped/types/cheerio at master · DefinitelyTyped/DefinitelyTyped · GitHub

すると、cheerio.ElementTagElementにのみattribsの型が定義されていることがわかった。
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/68e48c5ad46f0e7733feace0a9120d7dc3108462/types/cheerio/index.d.ts#L33

テストを見ても、elementtypetextではない、つまり、TextElementではなくTagElementであることを確認している。
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/b41bbafcd298b4b2aaa851d43ffadb6ef3565e69/types/cheerio/cheerio-tests.ts#L347

なので、それを適用して、自分で作ったanyを回収しにいった。
github.com

src/@types/*d.tsに型定義があるのと比べて、npmでインストールした場合は、node_modules/@types/*/*d.tsに型定義が隠蔽されてしまうので、便利だけど難しいなと思う。
それを解消する何かがもうすでにあるのだろうか。



TwitterでこのOSSの作者から感謝してもらえた。
https://twitter.com/ospfranco/status/1353312260823855105

(フォローしたけどフォローは返ってこなかった)

自分のPRを見て思う、TableDrivenTestsの良さ

jdという、commandline utility and Go library for diffing and patching JSON valuesなライブラリにTableDrivenTestsを導入したので、その良さを主観的にまとめておこうと思う。

github.com

github.com


まずは、実際にdiffの一部を見てみる。
https://github.com/josephburnett/jd/pull/29/files#diff-a0a78ab7b3426f60e7b758b565129777bdcef35e95a490999a0ff377389576d1R83

before

ctx := newTestContext(t)
checkDiff(ctx, `[]`, `[]`)
checkDiff(ctx, `[[]]`, `[[1]]`,
	`@ [0,0]`,
	`+ 1`)
checkDiff(ctx, `[1,2,3]`, `[1,null,3]`,
	`@ [1]`,
	`- 2`,
	`+ null`)
checkDiff(ctx, `[]`, `[3,4,5]`,
	`@ [0]`,
	`+ 3`,
	`@ [1]`,
	`+ 4`,
	`@ [2]`,
	`+ 5`)

after

ctx := newTestContext(t)
tests := []struct {
	context   *testContext
	a         string
	b         string
	diffLines []string
}{
	{ctx, `[]`, `[]`, []string {}},
        {ctx, `[[]]`, `[[1]]`, []string {
	     `@ [0,0]`,
	     `+ 1`,
	  },
        },
	{ctx, `[1,2,3]`, `[1,null,3]`, []string {
	    `@ [1]`,
	     `- 2`,
            `+ null`,
           },
        },
        {ctx, `[]`, `[3,4,5]`, []string {
            `@ [0]`,
	    `+ 3`,
            `@ [1]`,
            `+ 4`,
            `@ [2]`,
            `+ 5`,
	   },
	},
}
for _, tt := range tests {
     checkDiff(tt.context, tt.a, tt.b, tt.diffLines...)
}

よいと思うところを2つ書く。

関数呼び出しが冗長でなくなった

beforeafterで関数を呼び出す回数は変わらないが、afterはループの中で呼び出しているので、パッと見て冗長な表現にはなっていない。
func TestArrayDiff(t *testing.T) {のなかのテストなので、checkDiffを何回も呼び出している(ように見える)のは多少くどい。

関数の引数が宣言的になった

checkDiff関数は、func checkDiff(ctx *testContext, a, b string, diffLines ...string) {という形で引数を取る。
jd/common_test.go at d1d4edfee5375f88c1155ef55158597fb8488751 · josephburnett/jd · GitHub

beforeでは、どこからがdiffLinesが多少わかりにくくなっているが、afterでは、[]stringでまとめているため、checkDiffに何がどの引数で渡されるのかが分かりやすくなっている。

{
	context   *testContext
	a         string
	b         string
	diffLines []string
}

また、上のように、テストのはじめに宣言できることで、可読性が非常に高くなっている。
今回のようにbeforeがあの状態だったら、TableDrivenTestを導入しない手はないのではないかと思う。

割といい変更がだせたので自画自賛タイトルをつけてみた。