4

sigaction()のような、コードに誤りがない場合には失敗しない関数の、エラーチェックの実装方法について質問です。

例えば下記のようなコードだと、SIGUSR1のところに変な値を入れない(実装ミスがない)限り、printf("error");は実行されません。そのため、単体テストでprintf("error");を通るテストが書けず、カバレッジが100%にならないのでもやもやしています。

  ret = sigaction(SIGUSR1, &new_handler, &old_handler);
  if (0 > ret) {
    printf("error");
  }

そこで、下記のようにassert()関数で返り値をチェックしようと思うのですが、assert()関数の使いかたというか考え方として正しいでしょうか。

  ret = sigaction(SIGUSR1, &new_handler, &old_handler);
  assert (0 > ret);
1
  • 単体テストなら、テスト対象の関数以外はスタブにすべきなのでは?関連する関数(変数も)をスタブにしないのであれば、カバレッジ100%は不可能だと思います。
    – alpha
    Commented 2023年6月11日 8:41

2 件の回答 2

6

いくつかの問題が絡み合っていそうなので、複数の視点で書きます。

assert で書き換えて良いのか

assert で書き換える前と後のコードでは、挙動が異なります。前者は printf するだけですが後者は異常終了します。その変化を受け入れられるのかはチェックしておくべきです。

assert でも想定していた挙動を満たすのであれば、単なるコードの書き換えとしては特に問題ないものになります。

コードカバレッジ 100% を目指すべきなのか

状況にもよるのですが、必ずしもコードカバレッジは 100% ぴったりを目指さないといけないものではありません。その大変さの割に安全性はそこまで保証されないためです。

もし実際に気にされているコードも printf しているだけの単純なコードであったり、想定していない挙動をしてもさほど影響のないコードであったりするのであれば、わざわざこの条件文の中のカバレッジを気にするのは やりすぎかもしれません。

参考: The Case Against 100% Code Coverage -- Codecov

モックを利用できないか

ここでいう「テスト」が単体テストであるならば、テストフレームワークによっては関数のモックが利用できるでしょう。「モック」はフレームワークや使われ方によって「スタブ」「ダミー」「ダブル」などとも呼ばれます。

実際に引数を用意して sigaction の返り値が異常になる場合を作るのではなくて、必ず異常になるようなモックを用意して異常系のテストをする方がテストの実装を単純にできます。

5

assert で書き換えて良いのかに関しては、 nekketsuuu さんの回答にある通りと思います(NDEBUG が定義されていると消えてしまう点も注意が必要です)。assert で代替できず、防御的なコードを残した際にカバレッジが 100% にならない問題には、例えば以下で対処できます。

カバレッジ計測時には分岐コードを削除する方法

カバレッジを計測するテストの際には、条件部を定数の真理値にしてしまい、分岐コードが生成されないようにする方法が有ります。単純に考えると、ビルド時のフラグによって、

  • カバレッジ計測時: 条件部のコードを 0 にして分岐を削除する
  • その他のビルド時: 条件部のコードそのまま

の2通りのコードを出し分けるマクロを定義すればよいはずです。概念的には下のようなコードになります。

// これは説明のためのコードであり未テストです

#if defined(COVERAGE_TEST)
    #define NEVER(X) (0)
#else
    #define NEVER(X) (X)
#endif

// マクロの使用:
//   真にならないはずの条件を NEVER で囲む
if ( NEVER(0 > ret) ) {
    // リカバリー処理など
}

これは、SQLite で用いられている方法です。SQLite では、

  • リリース
  • 通常のテスト
  • テストカバレッジの測定

の3通りのコードを生成するマクロを用いています。

参照: How SQLite Is Tested / 7.2. Coverage testing of defensive code

該当のマクロ定義(sqliteInt.h):

/*
** The ALWAYS and NEVER macros surround boolean expressions which
** are intended to always be true or false, respectively.  Such
** expressions could be omitted from the code completely.  But they
** are included in a few cases in order to enhance the resilience
** of SQLite to unexpected behavior - to make the code "self-healing"
** or "ductile" rather than being "brittle" and crashing at the first
** hint of unplanned behavior.
**
** In other words, ALWAYS and NEVER are added for defensive code.
**
** When doing coverage testing ALWAYS and NEVER are hard-coded to
** be true and false so that the unreachable code they specify will
** not be counted as untested code.
*/
#if defined(SQLITE_OMIT_AUXILIARY_SAFETY_CHECKS)
# define ALWAYS(X)      (1)
# define NEVER(X)       (0)
#elif !defined(NDEBUG)
# define ALWAYS(X)      ((X)?1:(assert(0),0))
# define NEVER(X)       ((X)?(assert(0),1):0)
#else
# define ALWAYS(X)      (X)
# define NEVER(X)       (X)
#endif

この質問に回答するには、ログインする必要があります。

求めていた回答ではありませんか? のタグが付いた他の質問を参照する。