Apakah membangun objek dengan parameter nol dalam pengujian unit OK?

37

Saya mulai menulis beberapa unit test untuk proyek saya saat ini. Saya tidak benar-benar memiliki pengalaman dengannya. Pertama saya ingin sepenuhnya "mengerti", jadi saya saat ini tidak menggunakan framework IoC saya atau perpustakaan yang mengejek.

Saya bertanya-tanya apakah ada yang salah dengan memberikan argumen nol untuk konstruktor objek dalam unit test. Biarkan saya memberikan beberapa contoh kode:

public class CarRadio
{...}


public class Motor
{
    public void SetSpeed(float speed){...}
}


public class Car
{
    public Car(CarRadio carRadio, Motor motor){...}
}


public class SpeedLimit
{
    public bool IsViolatedBy(Car car){...}
}

Namun Another Car Code Example (TM), berkurang menjadi hanya bagian-bagian yang penting untuk pertanyaan. Saya sekarang menulis tes seperti ini:

public class SpeedLimitTest
{
    public void TestSpeedLimit()
    {
        Motor motor = new Motor();
        motor.SetSpeed(10f);
        Car car = new Car(null, motor);

        SpeedLimit speedLimit = new SpeedLimit();
        Assert.IsTrue(speedLimit.IsViolatedBy(car));
    }
}

Tes berjalan dengan baik. SpeedLimitperlu Cardengan Motoruntuk melakukan hal itu. Itu tidak tertarik CarRadiosama sekali, jadi saya berikan nol untuk itu.

Saya bertanya-tanya apakah suatu objek menyediakan fungsionalitas yang benar tanpa sepenuhnya dibangun adalah pelanggaran SRP atau bau kode. Saya memiliki perasaan yang mengganggu, tetapi speedLimit.IsViolatedBy(motor)tidak terasa benar - batas kecepatan dilanggar oleh mobil, bukan motor. Mungkin saya hanya perlu perspektif berbeda untuk unit test vs kode kerja, karena seluruh niatnya adalah untuk menguji hanya sebagian saja.

Apakah membangun objek dengan null dalam tes unit bau kode?

R. Schmitz
sumber
24
Agak di luar topik, tetapi Motormungkin seharusnya tidak memiliki speedsama sekali. Seharusnya throttledan menghitung torqueberdasarkan arus rpmdan throttle. Adalah tugas mobil untuk menggunakan Transmissionuntuk mengintegrasikan itu ke kecepatan saat ini, dan untuk mengubahnya menjadi rpmsinyal untuk kembali ke Motor... Tapi saya kira, Anda toh tidak ada di dalamnya untuk realisme, kan?
cmaster
17
Bau kode adalah bahwa konstruktor Anda mengambil nol tanpa mengeluh sejak awal. Jika menurut Anda ini dapat diterima (Anda mungkin punya alasan untuk ini): silakan, coba!
Tommy
4
Pertimbangkan pola Null-Object . Ini sering menyederhanakan kode, sementara juga membuatnya aman-NPE.
Bakuriu
17
Selamat : Anda telah menguji bahwa dengan nullradio, batas kecepatan dihitung dengan benar. Sekarang Anda mungkin ingin membuat tes untuk memvalidasi batas kecepatan dengan radio; kalau-kalau perilakunya berbeda ...
Matthieu M.
3
Tidak yakin dengan contoh ini, tetapi kadang-kadang fakta bahwa Anda hanya ingin menguji setengah kelas adalah petunjuk bahwa ada sesuatu yang harus dipecah
Owen

Jawaban:

70

Dalam kasus contoh di atas, masuk akal bahwa a Cardapat ada tanpa a CarRadio. Dalam hal ini, saya akan mengatakan bahwa tidak hanya dapat diterima dengan nol CarRadiokadang-kadang, saya akan mengatakan bahwa itu wajib. Tes Anda perlu memastikan bahwa implementasi Carkelas kuat, dan tidak membuang pengecualian null pointer ketika tidak CarRadioada.

Namun, mari kita ambil contoh berbeda - mari kita pertimbangkan a SteeringWheel. Mari kita asumsikan bahwa Carharus memiliki SteeringWheel, tetapi tes kecepatan tidak benar-benar peduli. Dalam hal ini, saya tidak akan memberikan nol SteeringWheelkarena ini kemudian mendorong Carkelas ke tempat-tempat di mana ia tidak dirancang untuk pergi. Dalam hal ini, Anda akan lebih baik membuat semacam DefaultSteeringWheelyang (untuk melanjutkan metafora) dikunci dalam garis lurus.

Pete
sumber
44
Dan jangan lupa untuk menulis unit test untuk memeriksa bahwa a Cartidak dapat dibangun tanpa SteeringWheel...
cmaster
13
Saya mungkin kehilangan sesuatu, tetapi jika itu mungkin dan bahkan umum untuk membuat Cartanpa CarRadio, tidak masuk akal untuk membuat konstruktor yang tidak memerlukannya untuk dilewati dan tidak perlu khawatir tentang null eksplisit sama sekali ?
an earwig
16
Mobil yang mengemudi sendiri mungkin tidak memiliki setir. Hanya mengatakan. ☺
BlackJack
1
@ James_Parsons Saya lupa menambahkan bahwa ini tentang kelas yang tidak memiliki pemeriksaan nol karena hanya digunakan secara internal dan selalu menerima parameter non-nol. Metode lain Carakan melempar NRE dengan nol CarRadio, tetapi kode yang relevan untuk SpeedLimittidak. Dalam hal pertanyaan seperti yang diposting sekarang Anda akan benar dan saya memang akan melakukannya.
R. Schmitz
2
@ mtraceur Cara semua orang berasumsi bahwa kelas yang mengizinkan konstruksi dengan argumen nol berarti nol adalah pilihan yang valid membuat saya menyadari bahwa saya mungkin harus memiliki pemeriksaan nol di sana. Masalah aktual yang saya miliki kemudian dibahas oleh banyak jawaban yang baik, menarik, dan ditulis dengan baik di sini yang tidak ingin saya hancurkan dan yang memang membantu saya. Juga menerima jawaban ini sekarang karena saya tidak suka meninggalkan hal-hal yang belum selesai dan ini adalah yang paling terangkat (walaupun mereka semua sangat membantu).
R. Schmitz
23

Apakah membangun objek dengan null dalam tes unit bau kode?

Iya nih. Perhatikan definisi kode bau C2 : "CodeSmell adalah petunjuk bahwa ada sesuatu yang salah, bukan kepastian."

Tes berjalan dengan baik. SpeedLimit membutuhkan Mobil dengan Motor untuk melakukan tugasnya. Sama sekali tidak tertarik dengan CarRadio, jadi saya berikan null untuk itu.

Jika Anda mencoba menunjukkan bahwa speedLimit.IsViolatedBy(car)tidak bergantung pada CarRadioargumen yang diteruskan ke konstruktor, maka mungkin akan lebih jelas bagi pengelola di masa mendatang untuk membuat batasan itu secara eksplisit dengan memperkenalkan uji ganda yang gagal dalam pengujian jika itu dipanggil.

Jika, seperti lebih mungkin, Anda hanya berusaha menyelamatkan diri dari pekerjaan membuat CarRadioyang Anda tahu tidak digunakan dalam kasus ini, Anda harus memperhatikan bahwa

  • ini merupakan pelanggaran enkapsulasi (Anda membiarkan fakta bahwa CarRadiotidak digunakan bocor ke dalam ujian)
  • Anda telah menemukan setidaknya satu kasus di mana Anda ingin membuat Cartanpa menentukan aCarRadio

Saya sangat curiga bahwa kode tersebut mencoba memberi tahu Anda bahwa Anda menginginkan konstruktor yang mirip

public Car(IMotor motor) {...}

dan kemudian konstruktor itu dapat memutuskan apa yang harus dilakukan dengan fakta bahwa tidak adaCarRadio

public Car(IMotor motor) {
    this(null, motor);
}

atau

public Car(IMotor motor) {
    this( new ICarRadio() {...} , motor);
}

atau

public Car(IMotor motor) {
    this( new DefaultRadio(), motor);
}

dll.

Tentang jika meneruskan nol ke sesuatu yang lain saat menguji SpeedLimit. Anda dapat melihat bahwa tes ini disebut "SpeedLimitTest" dan satu-satunya Assert yang memeriksa metode SpeedLimit.

Itu bau yang menarik kedua - untuk menguji SpeedLimit, Anda harus membangun seluruh mobil di sekitar radio, meskipun satu-satunya hal yang mungkin diperhatikan oleh pemeriksaan SpeedLimit adalah kecepatan .

Ini mungkin petunjuk yang SpeedLimit.isViolatedByharus menerima antarmuka peran yang diimplementasikan oleh mobil , daripada membutuhkan seluruh mobil.

interface IHaveSpeed {
    float speed();
}

class Car implements IHaveSpeed {...}

IHaveSpeedadalah nama yang sangat buruk; semoga bahasa domain Anda akan mengalami peningkatan.

Dengan antarmuka itu, pengujian Anda SpeedLimitbisa menjadi lebih sederhana

public void TestSpeedLimit()
{
    IHaveSpeed somethingTravelingQuickly = new IHaveSpeed {
        float speed() { return 10f; }
    }

    SpeedLimit speedLimit = new SpeedLimit();
    Assert.IsTrue(speedLimit.IsViolatedBy(somethingTravelingQuickly));
}
VoiceOfUnreason
sumber
Dalam contoh terakhir Anda, saya kira Anda harus membuat kelas tiruan yang mengimplementasikan IHaveSpeedantarmuka sebagai (setidaknya dalam c #) Anda tidak akan dapat membuat instance antarmuka itu sendiri.
Ryan Searle
1
Itu benar - ejaan yang efektif akan tergantung pada rasa Blub yang Anda gunakan untuk tes Anda.
VoiceOfUnreason
18

Apakah membangun objek dengan null dalam tes unit bau kode?

Tidak

Tidak semuanya. Sebaliknya, jika NULLnilai yang tersedia sebagai argumen (beberapa bahasa mungkin memiliki konstruksi yang melarang lewat dari pointer nol), Anda harus mengujinya.

Pertanyaan lain adalah apa yang terjadi ketika Anda lulus NULL. Tapi itulah yang dimaksud dengan unit test: memastikan hasil yang pasti terjadi ketika untuk setiap input yang memungkinkan. Dan NULL merupakan input potensial, jadi Anda harus memiliki hasil yang ditentukan dan Anda harus memiliki tes untuk itu.

Jadi jika Mobil Anda seharusnya dapat dibangun tanpa radio, Anda harus menulis tes untuk itu. Jika tidak dan seharusnya melempar pengecualian, Anda harus menulis tes untuk itu.

Either way, ya Anda harus menulis ujian untuk NULL. Itu akan menjadi bau kode jika Anda meninggalkannya. Itu berarti Anda memiliki cabang kode yang belum diuji yang baru saja Anda anggap ajaib bebas bug.


Harap dicatat bahwa saya setuju dengan jawaban lain bahwa kode Anda yang sedang diuji dapat ditingkatkan. Meskipun demikian, jika kode yang ditingkatkan memiliki parameter yang pointer, melewati NULLbahkan untuk kode yang ditingkatkan adalah tes yang baik. Saya ingin tes untuk menunjukkan apa yang terjadi ketika saya lulus NULLsebagai motor. Itu bukan bau kode, itu kebalikan dari bau kode. Ini pengujian.

tidak ada
sumber
Sepertinya Anda menulis tentang pengujian di Carsini. Meskipun saya tidak melihat apa pun yang saya anggap sebagai pernyataan yang salah, pertanyaannya adalah tentang pengujian SpeedLimit.
R. Schmitz
@ R.Schmitz Tidak yakin apa yang Anda maksud. Saya telah mengutip pertanyaan, ini tentang meneruskan NULLke konstruktor Car.
nvoigt
1
Ini tentang jika melewatkan nol ke sesuatu yang lain saat pengujianSpeedLimit . Anda dapat melihat bahwa tes ini disebut "SpeedLimitTest" dan hanya Assertmemeriksa metode SpeedLimit. Pengujian Car- misalnya "jika Mobil Anda seharusnya dapat dibangun tanpa radio, Anda harus menulis tes untuk itu" - akan terjadi dalam tes yang berbeda.
R. Schmitz
7

Saya pribadi akan ragu untuk menyuntikkan nol. Bahkan, setiap kali saya menyuntikkan dependensi ke dalam konstruktor, salah satu hal pertama yang saya lakukan adalah meningkatkan ArgumentNullException jika suntikan itu nol. Dengan begitu saya bisa menggunakannya dengan aman di dalam kelas saya, tanpa puluhan cek kosong yang tersebar. Inilah pola yang sangat umum. Perhatikan penggunaan readonly untuk memastikan dependensi yang diatur dalam konstruktor tidak dapat disetel ke nol (atau apa pun) setelahnya:

class Car
{
   private readonly ICarRadio _carRadio;
   private readonly IMotor _motor;

   public Car(ICarRadio carRadio, IMotor motor)
   {
      if (carRadio == null)
         throw new ArgumentNullException(nameof(carRadio));

      if (motor == null)
         throw new ArgumentNullException(nameof(motor));

      _carRadio = carRadio;
      _motor = motor;
   }
}

Dengan yang di atas, saya mungkin bisa memiliki satu atau dua unit test, di mana saya menyuntikkan nol, hanya untuk memastikan cek nol saya berfungsi seperti yang diharapkan.

Karena itu, ini menjadi masalah, setelah Anda melakukan dua hal:

  1. Program untuk antarmuka, bukan kelas.
  2. Gunakan kerangka kerja mengejek (seperti Moq untuk C #) untuk menyuntikkan dependensi yang diolok-olok alih-alih nol.

Jadi untuk contoh Anda, Anda akan memiliki:

interface ICarRadio
{
   bool Tune(float frequency);
}

interface Motor
{
   bool SetSpeed(float speed);
}

...
var car = new Car(new Moq<ICarRadio>().Object, new Moq<IMotor>().Object);

Rincian lebih lanjut tentang penggunaan Moq dapat ditemukan di sini .

Tentu saja, jika Anda ingin memahaminya tanpa mengejek terlebih dahulu, Anda masih bisa melakukannya, selama Anda menggunakan antarmuka. Cukup buat CarRadio dan Motor dummy sebagai berikut, dan suntikkan sebagai gantinya:

class DummyCarRadio : ICarRadio
{
   ...
}
class DummyMotor : IMotor
{
   ...
}

...
var car = new Car(new DummyCarRadio(), new DummyMotor())
Eternal21
sumber
3
Inilah jawaban yang akan saya tulis
Liath
1
Aku akan melangkah lebih jauh dengan mengatakan bahwa benda-benda tiruan itu juga harus memenuhi kontrak mereka. Jika IMotorpunya getSpeed()metode, maka DummyMotorperlu mengembalikan kecepatan yang dimodifikasi setelah berhasil setSpeedjuga.
ComFreek
1

Ini tidak hanya baik-baik saja, ini adalah teknik pemecahan ketergantungan yang terkenal untuk memasukkan kode lama ke dalam alat uji. (Lihat “Bekerja Secara Efektif dengan Kode Warisan” oleh Michael Feathers.)

Apakah baunya? Baik...

The tes tidak berbau. Tes sedang melakukan tugasnya. Itu menyatakan "objek ini bisa nol dan kode yang diuji masih berfungsi dengan benar".

Bau itu ada dalam implementasinya. Dengan mendeklarasikan argumen konstruktor, Anda menyatakan "kelas ini membutuhkan benda ini agar berfungsi dengan baik". Jelas, itu tidak benar. Apa pun yang tidak benar sedikit berbau.

Bebek karet
sumber
1

Mari kita mundur ke prinsip pertama.

Tes unit menguji beberapa aspek dari satu kelas.

Namun akan ada konstruktor atau metode yang mengambil kelas lain sebagai parameter. Cara Anda menangani ini akan bervariasi dari kasus ke kasus tetapi pengaturan harus minimal karena mereka tangensial ke tujuan. Mungkin ada skenario di mana melewati parameter NULL benar-benar valid - seperti mobil yang tidak memiliki radio.

Saya berasumsi di sini, bahwa kami hanya menguji garis balap untuk memulai (untuk melanjutkan tema mobil), tetapi Anda mungkin juga ingin meneruskan NULL ke parameter lain untuk memeriksa apakah kode pertahanan dan mekanisme penyerahan pengecualian berfungsi sebagai wajib.

Sejauh ini baik. Namun, bagaimana jika NULL bukan nilai yang valid. Nah, di sini Anda berada di dunia tiruan, bertopik, boneka dan palsu .

Jika semua ini tampaknya agak sulit, Anda sebenarnya sudah menggunakan dummy dengan memasukkan NULL di konstruktor Mobil karena nilai yang berpotensi tidak akan digunakan.

Apa yang seharusnya menjadi jelas dengan cepat, adalah bahwa Anda berpotensi menembak kode Anda dengan banyak penanganan NULL. Jika Anda mengganti parameter kelas dengan antarmuka, maka Anda juga membuka jalan untuk mengejek dan DI dll yang membuatnya jauh lebih mudah untuk ditangani.

Robbie Dee
sumber
1
"Tes unit adalah untuk menguji kode dari satu kelas." Huh . Bukan itu yang dimaksud dengan unit test dan mengikuti pedoman itu yang akan membawa Anda ke kelas-kelas yang membengkak atau tes-tes yang menghalangi dan produktif.
Semut
3
memperlakukan "unit" sebagai eufemisme untuk "kelas" sayangnya merupakan cara berpikir yang sangat umum tetapi pada kenyataannya yang dilakukan adalah (a) mendorong tes "sampah, sampah", yang benar-benar dapat merusak validitas seluruh tes suite dan (b) menegakkan batasan yang harus bebas untuk berubah, yang dapat melumpuhkan produktivitas. Saya berharap lebih banyak orang akan mendengarkan rasa sakit mereka dan menantang prakonsepsi ini.
Semut
3
Tidak ada kebingungan seperti itu. Uji unit perilaku, bukan unit kode. Tidak ada apa pun tentang konsep uji unit - kecuali dalam pikiran yang luas tentang pengujian perangkat lunak - yang menyiratkan bahwa unit uji digabungkan dengan konsep OOP kelas yang jelas. Dan kesalahpahaman yang sangat merusak juga.
Semut
1
Saya tidak yakin persis apa yang Anda maksudkan - saya berdebat sebaliknya tentang apa yang Anda maksudkan. Stub / tiru batas keras (jika Anda benar-benar harus) dan menguji unit perilaku . Melalui API publik. Apa itu API tergantung pada apa yang Anda bangun tetapi tapaknya harus minimal. Menambahkan abstraksi, polimorfisme atau kode restrukturisasi seharusnya tidak menyebabkan tes gagal - "unit per kelas" bertentangan dengan ini dan benar-benar merusak nilai tes di tempat pertama.
Semut
1
RobbieDee - Saya sedang berbicara tentang kebalikannya. Pertimbangkan bahwa Anda mungkin adalah orang yang menyimpan kesalahpahaman umum, mengunjungi kembali apa yang Anda anggap sebagai "unit" dan mungkin menggali lebih dalam apa yang sebenarnya dibicarakan oleh Kent Beck ketika ia berbicara tentang TDD. Apa yang kebanyakan orang sebut TDD sekarang adalah monstrositas kultus kargo. youtube.com/watch?v=EZ05e7EMOLM
Ant P
1

Melihat desain Anda, saya akan menyarankan bahwa a Carterdiri dari satu set Features. Fitur mungkin a CarRadio, atau EntertainmentSystemyang dapat terdiri dari hal-hal seperti CarRadio, DvdPlayerdll.

Jadi, minimal, Anda harus membangun Cardengan satu set Features. EntertainmentSystemdan CarRadioakan menjadi pelaksana antarmuka (Java, C # dll) dari public [I|i]nterface Featuredan ini akan menjadi contoh dari pola desain Komposit.

Oleh karena itu, Anda akan selalu membuat Cardengan Featuresdan unit test tidak akan pernah membuat Cartanpa Features. Meskipun Anda mungkin mempertimbangkan untuk mengejek seorang BasicTestCarFeatureSetyang memiliki serangkaian fungsi minimal yang diperlukan untuk pengujian paling dasar Anda.

Hanya beberapa pemikiran.

John

johnd27
sumber