Skip to content

Issue #91 remove googletest from contrib #116

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

Merged
merged 42 commits into from
Apr 1, 2024

Conversation

tsayukov
Copy link
Collaborator

@tsayukov tsayukov commented Mar 29, 2024

Issue #91

Добавил также CMakeLists файлы для юнит тестов внутри library/cpp/testing, чтобы проверить, что всё работает. Это временное решение, т.к. пока мне не ясно как у нас будут включаться тесты: через какой-нибудь ключ ENABLE_TESTING, который через if-ы будет всё включать (enable_testing() и остальное), или иным образом. По той же причине find_package(GTest) пока объявлен как REQUIRED, в идеале он должен искаться только, если включена поддержка тестирования.
Впрочем, задача была, чтобы библиотека собиралась, и сейчас она собирается.

Из интересного:

  • Думаю, что из-за легаси сборки через ya make некоторые вещи перестали работать корректно сейчас. Например, предполагаю, что раньше ya make самостоятельно устанавливал макро определения ARCADIA_ROOT и ARCADIA_BUILD_ROOT, как пути до верхнего уровня source- и build-файлов. В библиотеке так же был определён макрос __SOURCE_FILE__ (его реализация в util/system/src_root.h), который брал текущий путь до файла и обрезал у него префикс, если мог, так, чтобы получился путь, относительный верхнего уровня source- или build-файлов, в зависимости, где текущий файл находился. Из-за того, что первые макро определения отсутствовали, макрос ничего не обрезал, и это ломало остальные функции. Я исправил это, как мог, не трогая ARCADIA_ROOT и ARCADIA_BUILD_ROOT, если они определены, но на случай, если нет, вместо них подставляются пути из CMAKE_SOURCE_DIR и CMAKE_BINARY_DIR. Это починило падающие тесты.
  • Единственный тест, который до сих пор не работает, это получение корректного пути до gdb (library/cpp/testing/common/ut/env_ut.cpp, Runtime, GdbPath). С ним пока не получилось разобраться.

tsayukov added 29 commits March 29, 2024 14:36
Before replacement TString to std::string, the first one had operator bool, checking a string for emptiness
There's no guarantees that std::string::size_type will be always std::size_t
Old TMaybe had the second template parameter aka Policy, determining behavior of the value direct access if the value doesn't exist (throwing exception or doing something else). We don't need this template type parameter here since we always check optional for existence
* Rearrange std::string and std::string_view includes so that they are located in the group of the standard includes.
* Fix some operator ""s and ""sv, which demanded using directives such as std::string_literals and std::string_view_literals.
* Replace some explicit std::string and std::string_view constructors to operator ""s and ""sv.
It was used to include the header 'gtest_prod.h' from
contrib/restricted/googletest/include. With using imported targets from
GTest there's no need for that, because it's already included through
googletest's header 'gtest.h'.
'cpp-testing-common' library uses 'GetArcadiaSourcePath' function that's declared in 'scripts/c_templates/svnversion.h'. We need to use cmake function 'vcs_info' that runs the python script generating necessary C-file, and links that file with the library.
Since there's the special library for that purpose (library/cpp/svnversion), it's better to use it, and not some scripts folder using for file generation. At least 'library/cpp/getopt' is doing so.
This macros gets a path to the current file and strips the prefix that is either ARCADIA_ROOT or ARCADIA_BUILD_ROOT, i.e. the top source and binary directory. So the final path turns out to be relative.
But neither ARCADIA_ROOT or ARCADIA_BUILD_ROOT are defined. Probably, they were defined using ya make building system, but now it's not the case.
The current behavior breaks some unit tests, so there's the solution: using CMake top source and build path variables we can define helper macro definitions, which will be substituted instead of ARCADIA_[BUILD]_ROOT if those macros are not defined.
@tsayukov tsayukov requested a review from GitSparTV March 29, 2024 13:13
@tsayukov tsayukov requested a review from Gazizonoki April 1, 2024 21:06
@Gazizonoki Gazizonoki merged commit 5f47805 into main Apr 1, 2024
@Gazizonoki Gazizonoki deleted the issue-#91-remove-googletest-from-contrib branch April 1, 2024 21:54
Jlucblu pushed a commit that referenced this pull request Apr 4, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 5, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 5, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 5, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 5, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 5, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 5, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 5, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 9, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 9, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 9, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 9, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 9, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 24, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 24, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 24, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Jlucblu pushed a commit that referenced this pull request Apr 25, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Gazizonoki added a commit that referenced this pull request May 20, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Gazizonoki added a commit that referenced this pull request May 20, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Gazizonoki added a commit that referenced this pull request May 20, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Gazizonoki added a commit that referenced this pull request May 20, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
Gazizonoki added a commit that referenced this pull request May 20, 2024
---------

Co-authored-by: Bulat <[email protected]>
Co-authored-by: Bulat Gayazov <[email protected]>
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.

3 participants