Ubuntu Application Launcher

Merge lp://staging/~ted/ubuntu-app-launch/jobs-tests into lp://staging/ubuntu-app-launch

jobs-tests
Merge into trunk.17.04

Proposed by Ted Gould on 2016-11-10

Status:	Merged
Approved by:	Marcus Tomlinson on 2017-01-24
Approved revision:	286
Merged at revision:	279
Proposed branch:	lp://staging/~ted/ubuntu-app-launch/jobs-tests
Merge into:	lp://staging/ubuntu-app-launch
Prerequisite:	lp://staging/~ted/ubuntu-app-launch/signal-instances
Diff against target:	2286 lines (+691/-339) 16 files modified CMakeLists.txt (+22/-9) debian/control (+1/-0) libubuntu-app-launch/registry-impl.h (+1/-1) tests/CMakeLists.txt (+23/-17) tests/cgroup-reap-test.cc (+1/-1) tests/eventually-fixture.h (+57/-0) tests/exec-util-test.cc (+1/-1) tests/helper-test.cc (+25/-25) tests/jobs-base-test.cpp (+249/-0) tests/libual-cpp-test.cc (+83/-199) tests/libual-test.cc (+53/-47) tests/list-apps.cpp (+6/-6) tests/snapd-info-test.cpp (+10/-8) tests/snapd-mock.h (+1/-1) tests/spew-master.h (+142/-0) tests/zg-test.cc (+16/-24)
To merge this branch:	bzr merge lp://staging/~ted/ubuntu-app-launch/jobs-tests
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Marcus Tomlinson (community)			Approve on 2017-01-24
unity-api-1-bot	continuous-integration		Needs Fixing on 2017-01-23
dobey (community)		2016-11-10	Needs Fixing on 2017-01-12
Review via email: mp+310589@code.staging.launchpad.net

This proposal supersedes a proposal from 2016-10-26.

Commit message

Jobs interface specific tests

Description of the change

This adds some tests on the base instance object. I'd like this to expand to be on the manager object as well, but it looks like there is already coverage on them from the other tests. So going to avoid that in this branch as that'll be a larger refactor (started on it, but it is going to be a big branch, will put on its own MR)

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2017-01-09:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/154/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

dobey (dobey) on 2017-01-10:

review: Needs Fixing

Revision history for this message

Ted Gould (ted) wrote on 2017-01-11:

Merged the gmock-fixes branch into this one because basically everything was caused by this branch anyway. Will drop that one off the end of the silo. Fixes a bunch of comments.

lp://staging/~ted/ubuntu-app-launch/jobs-tests updated on 2017-01-11

274. By Ted Gould on 2017-01-11: Merge upstream changes
275. By Ted Gould on 2017-01-11: Backport the changes on the gmock-fix branch to here, where most of the problems were caused
276. By Ted Gould on 2017-01-11: Use a clearer definition of unsigned
277. By Ted Gould on 2017-01-11: Only or onto the transitional google-mock package
278. By Ted Gould on 2017-01-11: Alphabetize list

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2017-01-11:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/161/rebuild

review: Needs Fixing (continuous-integration)

lp://staging/~ted/ubuntu-app-launch/jobs-tests updated on 2017-01-12

279. By Ted Gould on 2017-01-12: Make build on 32-bit architectures where gsize is a different size
280. By Ted Gould on 2017-01-12: Make it so that tests can run in parallel without running over each other

Revision history for this message

dobey (dobey) wrote on 2017-01-12:

Some more in-line. Also, after removing launcher-static from coverage targets, I am getting the following error consistently from the failure-tests locally:

** (process:22885): CRITICAL **: Error parsing manifest for package 'com.test.good': com.test.good does not exist in any database for user test-user
terminate called after throwing an instance of 'std::runtime_error'
what(): Unable to get Click manifest for package: com.test.good
zsh: abort (core dumped) ./build/tests/failure-test

review: Needs Fixing

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2017-01-12:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/165/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Ted Gould (ted) wrote on 2017-01-13:

On Thu, 2017-01-12 at 23:07 +0000, Rodney Dawes wrote:
> > +STRING(TOLOWER "${CMAKE_BUILD_TYPE}" LOWER_CMAKE_BUILD_TYPE) +if
> > ("${LOWER_CMAKE_BUILD_TYPE}" STREQUAL "coverage") + set(CMAK
> > E_LINK_FLAGS "${CMAKE_LINK_FLAGS} -lgcov")
> Please replace tabs here with spaces, to be consistent with the rest
> of the file, and because this is not a Makefile. Also, can you
> elaborate on what exactly breaks with this block removed?
It was the snapd-info-test, but as per bellow, find changing that.

> > +endif()
> > +
> > enable_testing()
> >
> > pkg_check_modules(GLIB2 REQUIRED glib-2.0)
> > @@ -237,7 +239,22 @@
> > list(APPEND filter-list "${CMAKE_BINARY_DIR}/*")
> > endif()
> > ENABLE_COVERAGE_REPORT(
> > - TARGETS ubuntu-launcher
> > - TESTS application-icon-finder-test application-info-desktop-test
> > cgroup-reap-test exec-util-test failure-test helper-test helper-
> > handshake-test libual-test libual-cpp-test zg-test
> > - FILTER ${filter-list}
> > + TARGETS
> > + ubuntu-launcher
> > + launcher-static> >

> Having launcher-static here results in linker failures when I try to
> build this branch or any that depend on it, locally. Please remove
> launcher-static here, as it's not necessary.>

It is needed for several of the tests. It builds for me and in
Launchpad, so I don't think there's a build error caused by it.

> > + TESTS
> > + application-icon-finder-test
> > + application-info-desktop-test
> > + cgroup-reap-test exec-util-test
> > + failure-test
> > + helper-test
> > + helper-handshake-test
> > + jobs-base-test
> > + libual-test
> > + libual-cpp-test
> > + list-apps
> > +# snap-info-test> >

> Please uncomment this, as I don't think there's any reason this would
> be not built on the platforms we're landing this on. We don't need to
> support vivid in trunk any more, so no need to be overly cautious
> about avoiding snap support (need to get rid of all the ifdefs later,
> but doesn't need to be in this branch, or series of).>

Eh, WFM. r281
>

> Why do you have both gtest_main and ${GTEST_MAIN_LIBRARIES} all over
> the place here? Please just use ${GMOCK_LIBRARIES} in place of both,
> here and everywhere else you do this.>

Some of the tests use GMOCK and some don't. It seems like that's the
difference between the two variables, no?
>
>

On Thu, 2017-01-12 at 23:07 +0000, Rodney Dawes wrote:
> > +STRING(TOLOWER "${CMAKE_BUILD_TYPE}" LOWER_CMAKE_BUILD_TYPE) +if
> > ("${LOWER_CMAKE_BUILD_TYPE}" STREQUAL "coverage") +	set(CMAK
> > E_LINK_FLAGS "${CMAKE_LINK_FLAGS} -lgcov")
>  Please replace tabs here with spaces, to be consistent with the rest
> of the file, and because this is not a Makefile. Also, can you
> elaborate on what exactly breaks with this block removed?
It was the snapd-info-test, but as per bellow, find changing that.

> > +endif()
> > +
> >  enable_testing()
> >  
> >  pkg_check_modules(GLIB2 REQUIRED glib-2.0)
> > @@ -237,7 +239,22 @@
> >    list(APPEND filter-list "${CMAKE_BINARY_DIR}/*")
> >  endif()
> >  ENABLE_COVERAGE_REPORT(
> > -  TARGETS ubuntu-launcher
> > -  TESTS application-icon-finder-test application-info-desktop-test
> > cgroup-reap-test exec-util-test failure-test helper-test helper-
> > handshake-test libual-test libual-cpp-test zg-test
> > -  FILTER ${filter-list}
> > +  TARGETS
> > +  	ubuntu-launcher
> > +	launcher-static> >

> Having launcher-static here results in linker failures when I try to
> build this branch or any that depend on it, locally. Please remove
> launcher-static here, as it's not necessary.>

It is needed for several of the tests. It builds for me and in
Launchpad, so I don't think there's a build error caused by it.

> > +  TESTS
> > +  	application-icon-finder-test
> > +	application-info-desktop-test
> > +	cgroup-reap-test exec-util-test
> > +	failure-test
> > +	helper-test
> > +	helper-handshake-test
> > +	jobs-base-test
> > +	libual-test
> > +	libual-cpp-test
> > +	list-apps
> > +#	snap-info-test> >

Eh, WFM. r281
>

> Why do you have both gtest_main and ${GTEST_MAIN_LIBRARIES} all over
> the place here? Please just use ${GMOCK_LIBRARIES} in place of both,
> here and everywhere else you do this.>

Some of the tests use GMOCK and some don't. It seems like that's the
difference between the two variables, no?
> 
>

lp://staging/~ted/ubuntu-app-launch/jobs-tests updated on 2017-01-13

281. By Ted Gould on 2017-01-13: Remove -lgcov hack and add snapd-info-test to the list of tests
282. By Ted Gould on 2017-01-13: Uhg, wanted to delete not just comment out

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2017-01-13:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/167/rebuild

review: Needs Fixing (continuous-integration)

lp://staging/~ted/ubuntu-app-launch/jobs-tests updated on 2017-01-19

283. By Ted Gould on 2017-01-19: Update to trunk
284. By Ted Gould on 2017-01-19: Switch to the local socket to avoid conflicts

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2017-01-19:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/173/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2017-01-20:

When trying to build this MP stack I got build errors about missing headers for libwhoopsie-dev and liblibertine-dev, though CMake didn't complain about them being missing from my system. Could you add find_package(x REQUIRED) lines for these.

Otherwise, again, very impressed by the test coverage you're maintaining in this project Ted. Outstanding.

review: Needs Fixing

Revision history for this message

Ted Gould (ted) wrote on 2017-01-23:

On Fri, 2017-01-20 at 09:20 +0000, Marcus Tomlinson wrote:
> When trying to build this MP stack I got build errors about missing
> headers for libwhoopsie-dev and liblibertine-dev, though CMake didn't
> complain about them being missing from my system. Could you add
> find_package(x REQUIRED) lines for these.
Fixed r285
> > + cgroup-reap-test exec-util-test
> >

>
> Small miss: exec-util-test should be on a new line
>

Fixed r286

lp://staging/~ted/ubuntu-app-launch/jobs-tests updated on 2017-01-23

285. By Ted Gould on 2017-01-23: Make required libraries required
286. By Ted Gould on 2017-01-23: Fix listing of targets

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2017-01-23:

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/175/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2017-01-24:

Looks good

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes:

Indicator Applet Developers

Ted Gould