Можно ли улучшить мой первый юнит-тест с применением boost::test?

Question

[Дмитрий]

Прошу сделать code-review и посоветовать что-либо улучшить.


Приведенный ниже код unit-теста выполняет проверку корректность записи обновленного хидера в файл классом CustomImage32.

Алгоритм теста проста: Если значения в хидере изменить, записать в файл, то после последующего чтения они должны совпадать с теми что были заданы в хидере перед записью в файл.

struct Correct32Fixture
{
    Correct32Fixture()
        : outputfile( string(OUTPUT_FILES_DIR) + string("Correct32.bin") )
    {
        string sourcefile( string(TEST_SAMPLES_DIR) + string("Correct32.bin") );
        std::remove( outputfile.c_str() );
        copy_file( path(sourcefile), path(outputfile) );
        fileRaw =  new fstream( outputfile.c_str(), ios::in | ios::out | ios::ate | ios::binary );
        filePtr = InOutStreamPtr( fileRaw );
        templateImg.setStream( filePtr );
        templateImg.read();
    }
    ~Correct32Fixture()
    {
        if(fileRaw->is_open() )
        {
            fileRaw->close();
        }
        std::remove( outputfile.c_str() );
    }
    string          outputfile;
    fstream*        fileRaw;
    InOutStreamPtr  filePtr; // InOutStreamPtr это boost::shared_ptr<std::iostream>
    CustomImage32   templateImg;
};
BOOST_FIXTURE_TEST_CASE( testSuperHeaderChanghing, Correct32Fixture )
{
    BOOST_REQUIRE( templateImg.SuperHeader().boundary_field1 == 0x90 );
    BOOST_REQUIRE( templateImg.SuperHeader().boundary_field2[9] == 0 );
    // Arrange
    templateImg.SuperHeader().boundary_field1 = 0xE9;
    templateImg.SuperHeader().boundary_field2[9] = 0xE8E9;
    templateImg.write();
    // Act
    fileRaw->close();
    fstream* fileRaw2 = new fstream( outputfile.c_str(), ios::in | ios::ate | ios::binary );
    InOutStreamPtr filePtr2( fileRaw2 );
    CustomImage32 checkImg( filePtr2 );
    //Assert
    BOOST_CHECK( checkImg.SuperHeader().boundary_field = 0xE9 );
    BOOST_CHECK( checkImg.SuperHeader().boundary_field[9] = 0xE8E9 );
}

Answer 1

[prograholic]

Замечание насчет exception safety + RAII:
Если вы используете boost::shared_ptr, то логично создавать объекты с помощью std::make_shared, а не использовать для этого сырой указатель (fileRaw и fileRaw2):

filePtr = boost::make_shared<std::fstream>( outputfile.c_str(), ios::in | ios::out | ios::ate | ios::binary );

. Ну и выкинуть использование fileRaw и fileRaw2.

Код юнит-теста — это тоже код, поэтому лучше не использовать в тесте магических констант: 0xE9 и пр.
Вынесите их в отдельное пространство имен:

namespace consts
{
  const int ConstantName = 0xE9;
  const char OutputFileName [] = "Correct32.bin";
}

Естественно, при этом константам дать осмысленные имена.

Также, я бы не стал использовать в юнит-тесте работу с файлами, вместо этого я бы использовал std::streambuf для чтения/записи данных в памяти — но это возможно, если в вашей программе есть механизм для ввода/вывода не только через fstream

Comments

[Дмитрий]

Спасибо огромное!!!
0) Спасибо за boost::make_shared, я буст от силы 3 недели использую и пока нет таких знаний как у Вас
1) «Магические константы» 0xE9 к самой предметной области никакого отношения не имеют. Надо было «что-то» записать и это «что-то» проверить с прочитанным после записи. Этим «что-то» вполне могло быть вполне и 0xFF или любое другое значение. Это «что-то» относится к тесту. Поэтому мне показалось что излишне эти константы называть const uint8_t EtalonValue = 0xE9 но за спасибо за совет по вынесению в отдельное пространство имен.
2) Да, Вы вернозаметили, что в моей программе есть механизм ввода\вывода не только по средством fstream. На самом деле в конструкторе класса CustomImage32 я принимаю boost::shared_ptr<std::iostream> что дает мне возможность работать с любым наследником std::iostream в том числе и с std::fstream. Хотя сейчас задумываюсь о boost::iostream-ах или boost::asio, но эти улучшения чуть позже, сейчас нужно получить хоть какой-то но рабочий экземпляр кода