Pythonのtry-else句と、テストカバレッジ100%が必ずしも品質を保証しないという話

こんにちは。fastAPI歴○か月のケイ太です。

先日、テストカバレッジも大事だけれど、それだけで品質を保証できるわけではない、ということを実感することがありました。

きっかけはこんなコードです。

def create_users(db:Session, users_list: List[UserCreate]) -> List[Users]:
    try:
        new_users = [create_user_instance(user) for user in users_list]
        db.bulk_save_objects(new_users)
    except Exception as e:
        db.rollback()
        return f"An error occurred: {str(e)}"
    else
        db.commit()
    
    return users_instances    

ぱっと見、問題なさそう…?と思いましたが、少し気になるポイントが。db.commit()がelse句の中にあります。「これって本当に大丈夫なのかな?」と思って調べてみました。

else句はどういう時に使うのか確認すると、

8. エラーと例外 — Python 3.12.0 ドキュメント

try 文は下記のように動作します。 まず、 try 節 (try clause) (キーワード try と except の間の文) が実行されます。 何も例外が発生しなければ、 except 節 をスキップして try 文の実行を終えます。(中略)
try ... except 文には、オプションで else 節 (else clause) を設けることができます。 else 節を設ける場合、全ての except 節よりも後ろに置かなければなりません。 else 節は try 節 で全く例外が送出されなかったときに実行されるコードを書くのに役立ちます。

とあります。 db.commit()自体が例外を発生させる可能性があるのに、else句の中に置いてしまうと、予期しないエラーや不整合が起きる可能性がありそうです。db.commit()はtry句の中に書かないといけない、ということです。

このコードのように、例外処理やロジックが間違っていたりしても、カバレッジでは補足できません。コードの質やロジックの正しさに注意を向けずに、カバレッジの数字だけ追ってもあまり意味はないのですね。