Skip to content
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

[Not for merging] Fixes for library unit tests with gtest #292

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

tsayukov
Copy link
Collaborator

@tsayukov tsayukov commented Aug 11, 2024

Часть коммитов здесь затрагивает только CMake файлы, а самые последние коммиты – это с изменениями в library – скорее иллюстрация, как я предлагаю решить проблему. Перед финальным мёржем эти изменения собираюсь откатить.

Здесь все юнит-тесты из library, которые зависят от library/cpp/testing/gtest, а именно:

Я видел, что в тестах для library/cpp/coroutine/engine/stack эту зависимость поменяли: и теперь те юнит-тесты зависят напрямую от гугл-тест библиотеки, без прослойки library/cpp/testing/gtest. Здесь я предлагаю поступить так же, заменив инклюды #include <library/cpp/testing/gtest/gtest.h> на #include <gtest/gtest.h> и, где необходимо, #include <gmock/gmock.h>. Если неприемлемо, тогда нужно вернуть library/cpp/testing/gtest и library/cpp/testing/gtest_main.

Теперь к другим деталям:

  • В некоторых тестах добавил недостающие инклюды — это потребовалось после удаления инкдюда хедера из library/cpp/testing/gtest.
  • В library/cpp/yt/memory пока что закомментировал тест erased_storage_ut, потому что он требует недостающей библиотеки library/cpp/int128. Её нужно либо притащить, либо через макросы убрать часть тестов.
  • В library/cpp/yt/yson_string пока что закомментировал тест saveload_ut, потому что он требует недостающей библиотеки library/cpp/testing/gtest_extensions. У него только зависимость от макроса EXPECT_THROW_MESSAGE_HAS_SUBSTR. Наверное, лучше притащить эту либу, если не хотим переписывать тесты. Была ещё пара юнит тестов (раз и два), в которых были инклюды хедера из этой либы, но макросы из хедера не использовались, поэтому я эти инклюды убрал.
  • Падал тест library/cpp/yt/string/unittests/enum_ut.cpp, а именно, TFormatTest::Enum. Там просто тестировалась формат-функция, но в процессе TString присваивался пустой std::initializer_list, всё это попадало в std::string::assign(const char*, size_t), где уже грохалось в попытке разыменовать nullptr. Кажется, что это приколы исключительно libstdc++, потому что, как мне представляется, [nullptr, nullptr) – это вполне себе валидный диапазон, и единственное требование из стандарта к assign, чтобы он принимал валидный диапазон. Так или иначе, в library/cpp/yt/string/format-inl.h я заменил присваивание инишиалайзер листа на пустую строку, чтобы уж точно всё работало.

@tsayukov tsayukov changed the title [Not for merging] Fix library unit tests with gtest [Not for merging] Fixes for library unit tests with gtest Aug 11, 2024
@tsayukov tsayukov marked this pull request as ready for review August 11, 2024 23:45
Copy link
Collaborator

@Gazizonoki Gazizonoki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Замена инклюдов #include <library/cpp/testing/gtest/gtest.h>. Это мы будем делать при импорте library/cpp, так что с этим нет проблем
  • Тест yt saveload_ut. Вернем gtest_extensions, заведи issue
  • Тест yt erased_storage_ut. Вернем int128, заведи issue
  • Недостающие хедеры и баг с пустым std::initializer_list. Это я донесу к нам внутрь

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants