Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

動作テストの実装 〜 ヘルプ表示編 〜( Issue #14) #18

Merged
merged 20 commits into from
Jun 2, 2021

Conversation

KEINOS
Copy link
Member

@KEINOS KEINOS commented May 28, 2021

ドラフト〜

やること

  • 既存コマンドが引数なしの場合にエラー表示と status 1 で終了するか動作テストを ShellSpec でチェック
    • archive
    • check
    • checkkeylength
    • dec
    • enc
    • keygen
    • sign
    • verify
  • 作成したテストで出てきた不具合修正
  • テスト用のダミー秘密鍵&公開鍵の作成と設置 ./tests/.ssh
  • 実行スクリプトの作成 ./.github/run-test.sh
  • 実行スクリプトの GitHub Actions 登録(ランナー: Linux)

可能ならやりたいこと

  • macOS と Windows のランナー対応

やらないこと

  • 細かい動作検証やイレギュラー入力などをチェックするテスト
    • これらは、この PR を終えてから、以下のフローで対応することとします
      • Issue 立ち上げ → 再現テストの作成 → 修正
  • 既存スクリプトのリファクタ(関数化や高速化など)
  • 新機能の実装

Note

  • Draft が外れるまでは rebase & force pushmain ブランチの変更に追随します。

@KEINOS KEINOS self-assigned this May 28, 2021
@KEINOS KEINOS added the enhancement New feature or request label May 28, 2021
@KEINOS KEINOS force-pushed the feat-basic-test-issue-14 branch from 1b41c42 to ff2808a Compare May 29, 2021 01:15
@KEINOS
Copy link
Member Author

KEINOS commented May 29, 2021

う〜ん。基本動作のテストをしようにも色々と関数化しないとテストできないや。

例えば enc KEINOS ./sample.txt 時に https://github.com/KEINOS.keys から公開鍵を取得する箇所を関数化しないとモックしづらいし、環境変数にセットされていた場合にそれを利用するにしても、いささか煩雑な分岐が発生してしまう。

shellspec の「関数のモック」自体は簡単。例えば、以下のような関数があった場合:

unixtime() { date +%s; }

unixtime の中で使われる date の挙動を変えて色々試したい場合は、テスト内で関数をオーバーライドできる。

Describe 'unixtime return value'
  Mock date
    echo 1546268400
  End

  It 'should return the mocked value'
    When call unixtime
    The output should eq 1546268400
  End
End

Mocking @ shellspec.info より)

つまり、引数の KEINOS から該当する GitHub からの公開鍵を返す箇所を関数化して、テスト時にテスト用の公開鍵を返すようにモックできればガチャピンも喜ぶ、と。

なので、必要最低限な箇所の関数化もすることにしよう。

@yoshi389111
Copy link
Collaborator

yoshi389111 commented May 30, 2021

curl をモック化できればいい感じですかね

(関数のモックと同様にコマンドのモックもできるってどこかで見かけた気がするのですが、関数内限定ということなのかな)

@yoshi389111
Copy link
Collaborator

手元で curl のモックができないのか確認してみたのですが、モック可能のようです。

hello.sh:

#!/bin/bash
curl -s https://github.com/KEINOS.keys

spec/hello_spec.sh:

# shellcheck shell=sh
  
Describe "hello"
  Mock curl
    echo "hello"
  End

  It 'curl test'
    When run source ./hello.sh
    The output should eq 'hello'
  End
End

私、何か勘違いしてますかね?

@KEINOS
Copy link
Member Author

KEINOS commented May 30, 2021

curl をモック化できればいい感じ

O! M! G! 😱 そうじゃん!それ一発だけで結構テストできそうじゃん!さすが!!

こういうイメージよね。(バイナリで文字列比較ができるのかってのもあるけど)

Describe 'enc command'
  Mock curl
    echo [ここにダミーの公開鍵]
  End

  It 'should return the mocked value'
    When run enc KEINOS "/path/to/test/data.txt" "/path/to/encoded/data.enc"

    expect=$(cat "/path/to/test/data.enc")
    actual=$(cat "/path/to/encoded/data.enc")

    The path "/path/to/encoded/data.enc" should be file
    The variable "$expect" should equal "$actual"
  End
End

関数化とそのテストをはじめちゃったので、一旦それらをスタッシュして、cURL のモックで試してみる!!

ありがとうござい 💪 !!!

@KEINOS
Copy link
Member Author

KEINOS commented May 30, 2021

モック可能のようです。

おお、その動きだとできそうですね!!あざっす!!

@KEINOS KEINOS force-pushed the feat-basic-test-issue-14 branch from e4fa961 to 431bc3f Compare May 30, 2021 10:39
@KEINOS
Copy link
Member Author

KEINOS commented Jun 1, 2021

全部のコマンドのテストの実装を 1 つの PR でやるのは粒度が大きいので、一旦テストのベースだけを PR しようと思います。
各コマンドのテストは個別の PR でしたいと思います。

@KEINOS KEINOS changed the title 基本動作テストの実装( Issue #14) 動作テストの実装ヘルプ表示編( Issue #14) Jun 2, 2021
@KEINOS
Copy link
Member Author

KEINOS commented Jun 2, 2021

なんか、curl のモックがうまくいかないので、とりあえず「引数がない場合は、エラー表示とステータス 1 で終了するか」のテストだけ実装したので、先に進めたいと思います。今日の夕方にはドラフトを外して PR します。

全コマンドの基本テストは、わたし荷は重かったでごめんなさい。

KEINOS added 16 commits June 2, 2021 15:03
各コマンドが「引数なし」で呼ばれた場合に、ヘルプの表示とステータス 1  で終了するかのテスト。
* ed25519 と rsa (4096 bit) の鍵ペアで、パスフレーズ「あり」と「なし」の両方の 4 つを設置
* 古い OpenSSH のバージョン用は別途コミット
```shellsession
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
            @         WARNING: UNPROTECTED PRIVATE KEY FILE!          @
            @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
            Permissions 0644 for '/tmp/KEINOS..pub' are too open.
            It is required that your private key files are NOT accessible by others.
            This private key will be ignored.
```
- パス指定でコマンド実行に変更
@KEINOS KEINOS force-pushed the feat-basic-test-issue-14 branch from 3ca5ed2 to 0d000bd Compare June 2, 2021 06:09
KEINOS added 3 commits June 2, 2021 15:16
basic テストでなく arg がない場合のテストなので、ファイル名からわかるようにした。
@KEINOS KEINOS changed the title 動作テストの実装ヘルプ表示編( Issue #14) 動作テストの実装 〜 ヘルプ表示編 〜( Issue #14) Jun 2, 2021
@KEINOS KEINOS marked this pull request as ready for review June 2, 2021 08:21
@KEINOS
Copy link
Member Author

KEINOS commented Jun 2, 2021

コミット数多めで申し訳ありませんが、お手隙にレビューお願いします。m(_ _)m

@yoshi389111
Copy link
Collaborator

動きは問題ないと思います。

が、一つ気になるのは、秘密鍵っぽいファイルをアップロードしてあると、GitHub のセキュリティチェックで警告が出ないですかね?
(誤って秘密鍵を公開していると、GitHubが親切心から警告をしてくれるんじゃないかと思ったのです。そんなセキュリティチェックはないのかな?)

ともかく、LGTM です。

LGTM

@KEINOS
Copy link
Member Author

KEINOS commented Jun 2, 2021

GitHub のセキュリティチェックで警告が出ないですかね?

そうなんですよね。ソース内に埋め込みどころか、全部盛り盛りにファイルをぶっ込んでますもんね。

なんか IT 系ニュースとかで、それっぽい話を見た気もするのですが、公式には載ってなくて。サードパーティの GitHub App などはあるみたいなのですが。

なので、(今後テストに必要になるし)とりあえずマージして、警告が出たら OptOut する設定が出てくると思うので、報告したいと思います。

@KEINOS KEINOS merged commit 0c73489 into Qithub-BOT:master Jun 2, 2021
@KEINOS KEINOS deleted the feat-basic-test-issue-14 branch June 2, 2021 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants