Add tests for isolated builds #444
Conversation
mikepurvis
force-pushed
the
add-isolated-tests
branch
5 times, most recently
from
2cfa1b6 to
f5de6c1
Compare
| @@ -50,7 +48,8 @@ def create_tree_workspace(wf, build_type, n_pkg_layers, n_children=2): | |||
| for i in range(n_pkgs): | |||
| wf.create_package( | |||
| 'pkg_{}'.format(i), | |||
| depends=(['pkg_{}'.format(floor(i - 1) / n_children)] if i > 0 else [])) | |||
| build_type=build_type, | |||
| depends=(['pkg_{}'.format(int((i - 1) / n_children))] if i > 0 else [])) | |||
There was a problem hiding this comment.
This was previously ignoring the passed-in build_type, and the use of floor was resulting in dependencies like pkg_2.0.
| assert catkin_success(BUILD) | ||
| assert os.path.exists('devel/pkg_dep') | ||
| assert not os.path.exists('install') | ||
| assert_no_warnings(out) |
There was a problem hiding this comment.
Could condense these into one, but I'm not sure it's worth it from a readability standpoint.
mikepurvis
force-pushed
the
add-isolated-tests
branch
from
f5de6c1 to
a8b63ac
Compare
|
Good to go in now. Can confirm that this passes locally when rebased on top of the changes from #443. |
| """Test building dependent catkin packages with isolated installspace.""" | ||
| with redirected_stdio() as (out, err): | ||
| with workspace_factory() as wf: | ||
| n_pkgs = create_tree_workspace(wf, 'catkin', 2) |
There was a problem hiding this comment.
It's not possible to test plain CMake packages in this way, since the simplified factory-generated ones don't include CMake modules which make them findable by dependencies.
There was a problem hiding this comment.
That's true, unless you wanted to test something about a plain cmake package being a leaf. Also, despite that drawback, this function could be useful to get started and the test could add a CMake Module for the plain cmake package itself.
| cmake_minimum_required(VERSION 2.8.3) | ||
| project({name}) | ||
| find_package(catkin REQUIRED COMPONENTS {catkin_components}) | ||
| catkin_package()""" |
There was a problem hiding this comment.
There's a simplifying assumption here that a factory catkin package only depends on other catkin packages. All current tests pass with this assumption in place, so I see no reason to over complicate things.
There was a problem hiding this comment.
That's true, but at some point tests may just have to hand craft a CMakeLists.txt and not rely on a more and more complex function to generate everything for them.
| """Test building dependent catkin packages with isolated installspace.""" | ||
| with redirected_stdio() as (out, err): | ||
| with workspace_factory() as wf: | ||
| n_pkgs = create_tree_workspace(wf, 'catkin', 2) |
There was a problem hiding this comment.
That's true, unless you wanted to test something about a plain cmake package being a leaf. Also, despite that drawback, this function could be useful to get started and the test could add a CMake Module for the plain cmake package itself.
| assert catkin_success(BUILD) | ||
| assert os.path.exists('devel/pkg_dep') | ||
| assert not os.path.exists('install') | ||
| assert_no_warnings(out) |
| cmake_minimum_required(VERSION 2.8.3) | ||
| project({name}) | ||
| find_package(catkin REQUIRED COMPONENTS {catkin_components}) | ||
| catkin_package()""" |
There was a problem hiding this comment.
That's true, but at some point tests may just have to hand craft a CMakeLists.txt and not rely on a more and more complex function to generate everything for them.
|
I restarted travis, but not sure if this has to be rebased instead. |
|
The Linux jobs look ok, just waiting on Travis' macOS jobs to catch up. |
These should fail until #443 is merged.