LTであんまりふざけすぎると伝えたいことが伝わらないというはなし

以前勉強会でLTをしたが、ふざけすぎて伝えたいことがあまり伝わらなかったように思うのでメモ。

LT内容

「一年間コードレビューを受けてみて、どんなコメントがつくとつらかったか。それを解決するためにどうしたか。」ということを話した。
要約すると、

  • 静的解析のルールが甘かったので、静的解析で解決できる指摘をたくさん受けてしまった。現在は自分用のルールを追加して使っている。
  • 設計の大幅な変更の指摘を受けることがあった。AtomのTeletypeなどで、リアルタイムのレビューをして、設計段階からチームに共有することで解決できるのではないか。
  • 理由のない指摘があったので、レビュアーになるときはなぜよくないコードなのか論理的な説明をするように心がけようと思った。

ということを言いたかった。

反省点

「こんなコメントが付いて、それに対して自分はどう思う」ということを、おどけたような感じで発表した。
しかし、少しふざけて大げさにLTしたので、うまく伝わらなかったかもしれない。
具体的には、レビューがいつもつらい、いまもつらいと思われたような印象を受けた。
そんなことはなく、つらいこともあったが少しずつ改善していますよ、ということを伝えたかった。

学んだこと

懇親会含め話に出たことを箇条書きにする

  • コードレビューは運用するのがむずかしい
  • とくにチーム内にスキル差があるとむずかしい
  • 静的解析は大事、静的解析をとおったもののみコードレビューする
  • 静的解析のルールは古くなったり、過不足が出てくるので、定期的に見直す事が必要
  • コードレビューの目的を明確にする
  • プログラムを新規作成するときは、設計をドキュメントに残す
  • 新人がプログラムを新規作成するときは、ペアプログラミングなどで、設計段階からチームメンバーに共有するとよい
  • 複数人で開発しているとき、レビュアーはレビュイーAによくないコードの理由を説明しており、レビュイーBのレビューをおこなうときに、前に一度説明したと勘違いして理由を書いていないパターンがあるかもしれない

LT後の質疑応答で、進行予定の時間をオーバーするほどたくさんの意見が出た。コードレビューネタはみんな好きだし、盛り上がるなと感じた。

Visual Regression Testについてのメモ

Visual Regression Test(回帰テスト)を実施することになったので、自分用にメモしておく

Node.jsでWebアプリを開発している想定

Visual Regression Testとは

ビューに崩れがないか確認するテスト。 開発を進めていくと、意図しないビューの変更が加わる事があるので、それを防ぐことを目的とする。

以下は、自チーム用の方針。

いつおこなうか

毎日定時に実行

正のビューについて

ビューに崩れがないか確認する場合、比較対象が必要になる。これを正のビューと呼ぶこととする。 テストの流れは以下のようになる。 1.正のビューを用意する。 2.テスト対象のビューと正のビューを比較し差分を検出する。

  • 何を正のビューとするか
    本来ならばデザイン時に正のビューを作って用意することが望ましいが、現状ないので、最新バージョンのものを正のビューとする。
    この方法では、もし最新バージョンのビューがすでに仕様と異なっていた場合、Visual Regressinoテストでその間違いに気づくことができない。

  • デザインに変更がある場合は正のビューのデータを更新する必要がある。

  • 正のビューは画像として用意する
    HTML&CSSのテキストファイルとして用意する方法もあるらしいが、テスト対象のページがJavaScriptを使用しているページなので今回は除外。静的ページならその方法でもよさそう。

  • 動的要素をどのようにあつかうか
    同じページを表示しているのにバナーやカルーセルがあると、それらを差分と誤検知してしまう問題がある。
    これらに対する対策は、
    1.visibilityをhiddenにする。
    2.バナーやカルーセルにダミーのデータを用意する。
    などが考えられる。今回は、テスト用のライブラリなどに従い、visibilityをhiddenにする方向で行く。

検討過程

案1.定期実行

  • メリット 開発スピードに影響を与えない。

  • デメリット PRが複数取り込まれていたとき、デグレの原因がわからなくなる。 (実際に開発していると、ビューに影響があるPRかどうかはすぐにわかると思う)

  • 動作イメージ 定期的にビューのデータをS3に保存する。 保存したテスト対象のビューと正のビューを比較する。

案2.PRオープン時

  • メリット デグレの原因がPRに含まれていることがわかる。

  • デメリット スクショを撮るのに時間がかかるため、PRマージのスピードが遅くなる。

  • 動作イメージ PRオープン時に全てのビューをartifactsに保存する。 保存したビューと正のビューを比較し、その結果でCIをsuccess/failさせる。

以上を考えた上で、 ・デザインの変更は日常的に多くない ・スクリーンショットを撮るのでテストにある程度時間がかかる(開発のスピード感が失われる) ことを懸念して定時に実行することにした。

参考リンク

techblog.lclco.com

blog.kazu69.net

tech.recruit-mp.co.jp

babelでasyncを使おうとしたらregeneratorRuntime is not definedが出た

 babelでasyncを使おうとしたらregeneratorRuntime is not definedが出た

環境

"@babel/cli": "^7.0.0-beta.39",
"@babel/core": "^7.0.0-beta.39",
"@babel/preset-env": "^7.0.0-beta.39",
"@babel/register": "^7.0.0-beta.39",

対応

github.com こちらのissueにある下記コメントのように変更したら直った。 https://github.com/babel/babel/issues/5085#issuecomment-363242788
.bebelrc

{
  "presets": [
    [
      "@babel/preset-env"
    ]
  ]
}


.babelrc

{
  "presets": [
    [
      "@babel/preset-env", {
        "targets": {
          "node": "current"
        }
      }
    ]
  ]
}

に変更した。 babel-preset-envのドキュメントによると
https://github.com/babel/babel/tree/master/packages/babel-preset-env

For convenience, you can use "node": "current" to only include the necessary polyfills and transforms for the Node.js version that you use to run Babel:

必要なポリフィルとトランスフォームを含めてbabelを実行してくれるらしい

node-lambdaをes2017で書くまでの道のり

目的

aws lambdaのデプロイなどができるパッケージnode-lambdaでasync/await(es2017)を書けるようになる
プログラミングモデル (Node.js) - AWS Lambda

AWS Lambda は、現在以下の Node.js ランタイムをサポートしています。
* Node.js ランタイム v6.10 (ランタイム = nodejs6.10)

とあるように aws lambdaのNode.jsサポート環境は2018/02現在で6系までとなっており、async functionに対応していない

開発環境

$ node -v
v8.9.3
$ npm -v
5.6.0

テストフレームワークはmochaを使う

Babel7を使う

babelのGitHubリポジトリはベータ版である7系で説明されており、webページだと6系をつかって説明されている。ここの差異ですごくハマった。
「@babel/hogeのようにインストールするのがナウい」と聞いて躍起になって7系を使おうとしてしまったが、冷静に考えたら6系でよかった。
なぜベータ版に必死になってしまったのか。
ということでbabel7系を使う場合はGitHubリポジトリを読みながら進めるのがよい。公式のDocやググった情報を参考にするとハマる(ハマった)。

package.jsonに入れるもの

サポートされている環境に基づいて必要なプラグインとポリフィルを自動的に決定しますとのことなので、babel-preset-envを入れる。
(es2017対応環境で使うとそのままのコードが吐き出されて、トランスパイルされてないと思って焦った。es2015対応環境で使えばちゃんとそこで動くようにトランスパイルされる。)

npm-scriptsを書く

必要なパッケージをインストールしたらnpm-scriptsを書く

  "scripts": {
    "test": "mocha --recursive",
    "compile": "babel src --out-dir compiled",
    "package": "node-lambda package -H /compiled/index.handler",
    "deploy": "node-lambda deploy -H /compiled/index.handler"
  },

各コマンドを説明する

  • "test": "mocha --recursive"
    mocha.optsでbabel-registerを指定することでes2017で書いたテストを実行できる。 test/mocha.opts
--require @babel/register

mocha.optsを書かずに、"test": "mocha --recursive --require @babel/register"としても可。

  • "compile": "babel src --out-dir compiled"
    これでsrc/に入っているコードをトランスパイルしてcompiled/に出力する。

  • "deploy": "node-lambda deploy -H /compiled/index.handler"
    これでトランスパイルしたコードをlambdaのハンドラに指定してデプロイする。
    npm run packageも同様。 https://github.com/motdotla/node-lambda#deploy

    -H, --handler [index.handler] Lambda Handler {index.handler}

まとめ

これでlambdaでasync/awaitを使うことができるようになった。 ポイントだなと思ったことはトランスパイルしたコードをlambdaのハンドラに指定すること。
今まで見てきたlambdaのディレクトリ構成がlambda/index.js,lambda/lib/lib.js(index.jsでインポートされている)みたいな構成だったので、
webpackいるのかと思って悩んだけど、babelだけで書けた。

package.json

  {
  "scripts": {
    "test": "mocha --recursive",
    "compile": "babel src --out-dir compiled",
    "package": "node-lambda package -H /compiled/index.handler",
    "deploy": "node-lambda deploy -H /compiled/index.handler"
  },
  "devDependencies": {
    "@babel/cli": "^7.0.0-beta.39",
    "@babel/core": "^7.0.0-beta.39",
    "@babel/preset-env": "^7.0.0-beta.39",
    "@babel/register": "^7.0.0-beta.39",
    "chai": "^4.1.2",
    "mocha": "^5.0.0",
  }
}

create-react-appコマンドで作ったリポジトリをGitHubに上げる方法

ReactやVueなどのフレームワークcliから使う時、プロジェクトごと作ってくれる。

create-react-app my-app
create-react-native-app AwesomeProject
vue init webpack my-project

これをGitHub上にアップロードする方法をメモしておく。

  1. New repositoryでリポジトリを作る
  2. この時Initialize this repository with a READMEをチェックしない
  3. するとリポジトリを作った後に次に実行すべきコマンドが表示される
    …or push an existing repository from the command line にしたがいコマンドを実行する。 ローカルリポジトリを作ってリモートリポジトリを登録してからプッシュするという流れになる。

ローカルリポジトリで以下の手順を実行する。

createt-react-app my-app
cd my-app
git init
git remote add origin git@github.com:YOUR_USER_NAME/YOUR_REPOSITORY_NAME.git
git push -u origin master

コードを理解するときに個人的に気をつけているポイント

 

コードを読む時に気をつけているポイントの自分用メモ。(Githubを使っていることを想定)

 

ディレクトリ構成を理解する

ディレクトリ構成とファイル名を理解することでどんなプログラムか読み取ることができる。

フレームワークを使用している場合は、公式リファレンスにディレクトリの説明が書いてあるのでそれを読んで、ディレクトリの役割を理解する。

 

テストを読む

テストを読むことでインプットとアウトプットを理解する。

テストの文言からどのような事をおこなうプログラムなのか理解する。

 

コードを読む

ざっと関数だけを眺める。細かい実装は必要になったときに読む。

 

commitメッセージを読む

git blameを実行することで行毎のcommitメッセージを読むことができる。

git logを実行することでcommitログを新しい日付順に表示できる(デフォルトの場合)。

commitログからどのような変更か理解する。(Howの理解)

 

プルリクエストを読む

プルリクエストからどういった意図でプログラムが実装されているか理解する。(Whyの理解)

 

 

 

ログインをともなうE2Eテストのコツ

ここ数週間ログインをともなうE2Eテストを実装していたので、 そこで得られた学びをロギングしておく。

環境

Node.js v6.11.0
Nightmare v2.10.0
Mocha v3.4.0

コツ

  1. テストはリトライさせる
    E2Eテストは、ネットワークやディスクIOなど外部要因が絡んでくる。 ユニットテストと比較し、実行時間もかかるし、外部要因によってテストが失敗したりと、テスト結果が安定しない。そのため、テストに失敗したらリトライするような仕組みを用意する。mochaの場合はretryオプションがあるのでそれを使った。E2Eのためのオプションらしい。
    RETRY TESTS

    This feature is designed to handle end-to-end tests (functional tests/Selenium…) where resources cannot be easily mocked/stubbed.

  2. beforEachはリトライ対象にならない
    上記リトライオプションだが、mochaのbeforEachには適用されない。 参考issue
    beforeEachなどはあくまでsetup/teardownに使ってくれ。フックで安定しないロジック使ってるなら、それを関数化してくれ。ってことらしい。
    今回、ログインをともなうE2Eだったので、beforEachでログイン処理をしていたが、関数化して、ログイン処理をリトライに対応させた。
    最初、beforEachもリトライ対象になると思い込んでいて時間のロスをしてしまった。

  3. テストシナリオの各工程にはフィードバックを
    たとえば、「ログアウトできること」というテストシナリオの場合、ログイン>ログアウトという工程を経ることになる。ログインにはフォーム入力、ボタンクリックなど、ログアウトにはログアウトのボタンクリックなど、各工程にも作業がある。つまり、一つのテストシナリオは複数の工程から構成される。今回の案件の場合は10~20程度の工程があった。その各工程で、フィードバックがないとテストでコケたときに、原因特定に時間がかかってしまう。
    実際にあったのは、「ログアウトできること」というテストシナリオで、アサーションが「ログイン時に付与されるクッキーが削除されていること」だったのだが、自分の書いたテストは、ログインできていないのに、次の工程に進んでしまっていた。当然ログインしていないので、ログイン時に付与されるクッキーは無い。そのため、アサーションも通ってしまっていた。
    結局、別のテストが落ちることで、ログインできていないことが判明したが、テストが落ちる原因特定にかなり時間をロスしてしまった。(そして、そもそもログインできていない原因は、上記にあげた外部要因によるものだった。)

まとめ

ブラウザオートメーションはphantom.jsとかでやったことがあったし、ユニットテストも書いたことがあったが、E2Eテストは初めてだった。
感想としては、自動化はできるが、ユニットテストと違って時間もかかるので、安定させるのが難しく、テクニックを要するなぁという感じ。
しかし、E2Eテストで事前に検知できたバグもあるので、書いたかいがあった。手動で検証するよりは、繰り返し実行できるし、時間もかからないので書く価値は有ると思う。ただ、安定稼働にコストも割くので、テストを書くのはコアな機能に限定するなど、見極めが必要だとも感じた。