-
Notifications
You must be signed in to change notification settings - Fork 3
Rubets 07 filecopier #36
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
base: rubets_master
Are you sure you want to change the base?
Conversation
| .WillOnce(testing::Return(true)); | ||
| } | ||
|
|
||
| void BeforeCopyFileTest(const std::string& srcFolder, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
название не очевидное
| EXPECT_NO_THROW(fileCopier.Copy(s_srcFolder, s_dstFolder)); | ||
| } | ||
|
|
||
| TEST(FileCopierTests, Copy_FolderWithoutFiles) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
я не увидел, чтобы тест был красным.
Здесь или последовательность тестов неправильная или тест явно лишний
| EXPECT_NO_THROW(fileCopier.Copy(s_srcFolder, s_dstFolder)); | ||
| } | ||
|
|
||
| TEST(FileCopierTests, Copy_FileAndFolderWithFile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а где красный тест?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Посути, этот тест делает то, для чего уже были красные и зеленые тесты. Только здесь идет проверка, что мы скопируем как файлы в текущей папке так и из другой папки, находящейся в этой.
С одной стороны то что он желает уже покрыто, но нам также нужно знать что он выполняет и эти функции.
| const std::string dstPath(ConcatPath(dst, relativePath)); | ||
| if (m_fileSystem->IsDirectory(srcFilePath)) | ||
| { | ||
| Copy(srcFilePath, dstPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
меня смущает то, что если хотя бы один файл из иерархии будет отсутствовать, то мы не скопируем дальше ничего
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Идет проверка не существования файла, а существования папки. Ну и кроме проверки при первом вызове копирования, она больше не нужна. Следовало бы вынести функционал копирования в отдельную функцию.
No description provided.