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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[github] Execute drape_tests in CI #8205

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ferenc-
Copy link
Contributor

@Ferenc- Ferenc- commented May 17, 2024

No description provided.

@Ferenc- Ferenc- requested review from biodranik and vng May 17, 2024 14:42
endif()

target_link_libraries(${PROJECT_NAME}
qt_tstfrm
indexer # For StyleReader in static_texture_tests
drape
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change and source list changes needed? It looks unnatural and complex for cmake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you want to run all the tests, including those that rely on mocks in drape_tests/gl_functions.cpp, you can't link drape because, that would violate the one definition rule. Hence we don't link drape here, and also change to sources to not inlcude the original drape/gl_functions.cpp but the mock drape_tests/gl_functions.cpp instead.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation! Would it be clearer to use #ifdef in the mentioned files?

Copy link
Member

Choose a reason for hiding this comment

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

Can test's cmake file remove/replace a source from another target library

drape/drape_tests/font_texture_tests.cpp Outdated Show resolved Hide resolved
drape/drape_tests/vertex_buffer_tests.cpp Outdated Show resolved Hide resolved
Signed-off-by: Ferenc G茅czi <ferenc.gm@gmail.com>
@@ -90,7 +90,8 @@ UNIT_TEST(VertexBuffer_Benchmark)
"boost::small_vector time:", t3, "reserved vector time:", t4));
TEST_LESS(t2, t1, ());
TEST_LESS(t3, t2, ());
TEST_LESS(t4, t3, ());
// TODO: Fix this condition
//TEST_LESS(t4, t3, ());
Copy link
Member

Choose a reason for hiding this comment

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

This issue looks fishy to me. @vng @renderexpert ?

while (index.GetPendingNodesCount() < count)
;
// TODO: Fix this condition
//while (index.GetPendingNodesCount() < count)
Copy link
Member

Choose a reason for hiding this comment

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

This issue looks fishy to me. @vng @renderexpert ?

endif()

target_link_libraries(${PROJECT_NAME}
qt_tstfrm
indexer # For StyleReader in static_texture_tests
drape
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation! Would it be clearer to use #ifdef in the mentioned files?

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.

None yet

2 participants