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

CI testing of post install libraries #75

Closed
headupinclouds opened this issue Sep 25, 2016 · 12 comments
Closed

CI testing of post install libraries #75

headupinclouds opened this issue Sep 25, 2016 · 12 comments
Assignees

Comments

@headupinclouds
Copy link
Collaborator

We need to ensure that functionality in the post installation libraries (after stripping etc.) is tested, as something could go wrong during that stage.

@headupinclouds headupinclouds modified the milestones: 0.1, 0.2 Sep 25, 2016
@headupinclouds headupinclouds mentioned this issue Sep 30, 2016
4 tasks
@headupinclouds headupinclouds self-assigned this Sep 30, 2016
@headupinclouds headupinclouds changed the title CI testing of post build libraries CI testing of post install libraries Nov 10, 2016
@ruslo
Copy link
Collaborator

ruslo commented Nov 11, 2016

I've added "Testing of installed libraries" section here: #88. Will think about workaround.

@headupinclouds
Copy link
Collaborator Author

headupinclouds commented Nov 11, 2016

Thanks. It would be reassuring to make sure the builds work after packing. I think this could catch issues such as RPATH and symbol stripping errors.

[UPDATE] I see you already have enumerated some potential issues.

@ruslo
Copy link
Collaborator

ruslo commented Nov 13, 2016

Thinking about this feature. Also I will call it "integration test", though not sure about the correctness of this term :)

Got next points so far:


We can't use the same way of testing as usual and change only one option like DRISHTI_TEST_INSTALLED=ON:

if(DRISHTI_TEST_INSTALLED)
  find_package(drishti CONFIG REQUIRED)
else()
  add_subdirectory(...) # -> target 'drishti'
endif()

because in this case we have to watch really carefully the surrounding code. For example the find_package call may mask the missing find_dependency directive in dristhiConfig.cmake file. My opinion is that this approach is unmaintainable on practice.


Hence, the part of the project we want to run integration test with, must be stand-alone project:

cmake_minimum_required(VERSION x.y)
project(dristhi-tests)

if(TARGET drishti) # build as part of the project
  add_library(dristhi::drishti ALIAS drishti)
else()
  find_package(drishti CONFIG REQUIRED)
endif()

In this case we need to install dristhi locally first, then run build of such subproject and pass CMAKE_PREFIX_PATH with the value of locally installed dristhti package:

> build.py --toolchain abc --verbose --config Debug --install # install `drishti` locally
> build.py --toolchain abc --verbose --config Debug \
  --integration-test path/to/subproject

--integration-test will apply:

  • use path/to/subproject as a home (i.e. -Hpath/to/subproject)
  • need to use different binary directory to avoid conflicts with _builds/abc-Debug of the main project; let it be _builds/integration-test/abc-Debug
  • pass CMAKE_PREFIX_PATH=_install/abc
  • apply --test?

@headupinclouds
Copy link
Collaborator Author

headupinclouds commented Nov 13, 2016

Thanks for the overview. To complicate things further, I'm realizing that only HOST=TARGET builds are currently tested. Maybe Amazon Device Farm will work as a long term cloud solution (#58) for mobile device testing, but in the near term I think we could consider running the required tests if a suitable device is connected to the machine. Initially, this could be limited to iOS (ios-deploy) and Android (adb). I believe you have already added a CMake module to launch device targets on Android devices, so this would effectively be a for loop that would launch each CTest on the phone and check the status. This is a bit complicated, but it will be really nice to get to the point where each packaged SDK is fully tested on the platform it will be used. (Maybe device testing is best managed by a special python script?) I'm only just getting the basic test coverage in place for the library itself. I'm thinking maybe coveralls (#169) might be useful to have some visible badge indicating the extent of coverage.

Also I will call it "integration test", though not sure about the correctness of this term :)

This sounds good to me.

Re: hunter feature https://github.com/ruslo/hunter/issues/274. FWIW, this is relevant to the current CMake copy stage in the drishti CMake code here.

https://github.com/elucideye/drishti/blob/master/src/lib/drishti/CMakeLists.txt#L279-L290

Currently the shared library swallows most of the dependencies, so an end user (not using hunter) doesn't need them. In practice, this is complicated by the fact that the user application can have common dependencies, so these libraries (or at least the common ones) are shared with a release (including *.cmake package config files) in a 3rdparty folder.

@ruslo
Copy link
Collaborator

ruslo commented Nov 14, 2016

I'm realizing that only HOST=TARGET builds are currently tested

I think it's a separate issue. The feature I've described is about installing drishti and reusing installed libraries in subproject. What you're planning to do further is up to you. You can just configure such project and check that drishtiConfig.cmake is correctly written. You can then build the code and check that public defines propagated correctly. Or ever run tests to be 100% sure about functionality.

Initially, this could be limited to iOS (ios-deploy) and Android (adb).

Currently the shared library swallows most of the dependencies

This may increase the size of the final user application. Say, if some part of boost is fused into your shared library and user want to use boost too, then final executable will duplicate some boost code. We hit related issue already and I've discussed it here. After this discussion I decide to declare the linking static into shared the bad practice :) I will add examples and detailed description to CGold.

@ruslo
Copy link
Collaborator

ruslo commented Nov 14, 2016

Looks like we have to delegate this stuff to Hunter

Also we can assume that we are creating something that can work outside of the Hunter. And put the responsibility of correctness to the user.

@ruslo
Copy link
Collaborator

ruslo commented Nov 14, 2016

It looks like #75 is a longish task

-> 0.3

@ruslo ruslo modified the milestones: 0.3, 0.2 Nov 14, 2016
@headupinclouds
Copy link
Collaborator Author

Reading through this again.

After this discussion I decide to declare the linking static into shared the bad practice :)

I think it depends on the details of your use case. For many people running IDE project based development, I think a shared library (or dynamic framework) could be the preferred option (provided there are no conflicts, etc) if it provides a single convenient interface to work with. I don't see this is a good vs bad issue.

@ruslo
Copy link
Collaborator

ruslo commented Dec 20, 2016

For many people running IDE project based development, I think a shared library (or dynamic framework) could be the preferred option

Probably yes. By "I decide to declare the linking static into shared the bad practice" I mean in CGold project. CGold is about something minimal, correct and without "tricky" stuff. Also CGold is about approaches supported with CMake. All this public/private visibility which depends on the C++ code in fact and have no automatic control, this looks like a hell for the user who are not awared of such peculiarities. Also IDE + manual copy of framework is not about CMake-flow, hence out of CGold scope anyway.

PS I think using shared libraries for the 3rd parties (i.e. linking shared to shared) doesn't have such issue.

@headupinclouds
Copy link
Collaborator Author

headupinclouds commented Jan 22, 2017

Per comments above #75 (comment), we can start with a simple find_package() integration test in a second build stage here: ${DRISHTISDK}/src/integration/drishti-test/drishti-test.cpp.

Eventually, as drishti becomes a proper hunter-package, this can also support a hunter_add_package(drishti) interface. The initial test will look very similar to ${DRISHTISDK}/src/app/eye/eye.cpp.

Note: The public SDK interface to the drishti/hci module also needs to be completed and added.

@headupinclouds
Copy link
Collaborator Author

headupinclouds commented Jan 25, 2017

Initial build test in #282. There is no automated testing/deployment component to this yet -- just the build test. I think that should be a fairly straightforward extension of the internal drisht_add_test module (or some variation of that), with the exception of ios, which is using a dynamic framework. The current ${DRISHTISDK}/src/examples/integration/CMakeLists.txt builds the iOS test fine, but AFAIK, running the program on a device requires a framework embedding step that must be performed through the Xcode environment.

@headupinclouds headupinclouds removed this from the 0.3 milestone Feb 7, 2017
@headupinclouds
Copy link
Collaborator Author

Installed package integrity is now tested nicely through optional GIT_SELF use in the relocatable src/examples tree. The feature is there, which addresses the main concern of the original issue. We could use this at some point in the future for CI testing if we want to spend the CPU cycles.

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

No branches or pull requests

2 participants