Skip to content
This repository has been archived by the owner on Feb 4, 2020. It is now read-only.

updated ament_cmake's on_test for Windows #24

Merged
merged 1 commit into from Mar 31, 2015
Merged

Conversation

wjwwood
Copy link
Contributor

@wjwwood wjwwood commented Mar 28, 2015

This pull request changes the on_test code for the ament_cmake build type so that it can run tests on Windows using msbuild and the RUN_TESTS VS project.

Ready for review.

Fixes #23

@wjwwood wjwwood added the in progress Actively being worked on (Kanban column) label Mar 28, 2015
yield BuildAction(prefix + [MAKE_EXECUTABLE, 'test'])
else:
self.warn("Could not run test for ament_cmake package because it "
"has no 'test' target")
Copy link
Contributor

Choose a reason for hiding this comment

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

run test should be plural and I would suggest to quote the build type:

self.warn("Could not run tests for 'ament_cmake' package because it "
          "has no 'test' target")

It could also following the new 100 chars wrapping rule instead of at 80 chars. (Same below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's too long for 100 char without wrapping.

@dirk-thomas
Copy link
Contributor

The osrf_pycommon package does not pass the syntax check with Python 2.x. Should we remove Python 2.x from the .travis file?

@tfoote
Copy link
Member

tfoote commented Mar 28, 2015

Looks like the right behavior.

@dirk-thomas
Copy link
Contributor

Now the windows job (http://54.183.26.131:8080/view/ros2/job/ros2_batch_ci_windows/82/console) fails when trying to run the test command. The following Python code needs to handle Visual Studio:

output = subprocess.check_output([MAKE_EXECUTABLE, '-pn'], cwd=path)

@wjwwood
Copy link
Contributor Author

wjwwood commented Mar 31, 2015

Now the windows job (http://54.183.26.131:8080/view/ros2/job/ros2_batch_ci_windows/82/console) fails when trying to run the test command.

That's the original error I believe, the Windows job is not setup to test this branch. I'll change the Windows job to use this branch.

@wjwwood
Copy link
Contributor Author

wjwwood commented Mar 31, 2015

I've got a job running which is using this pr: http://54.183.26.131:8080/job/ros2_batch_ci_windows/83/console We'll have to see if that works.

@wjwwood
Copy link
Contributor Author

wjwwood commented Mar 31, 2015

It now fails with:

CMake Error at cmake/ament_copyright.cmake:19 (message):
  ament_copyright() could not find program 'ament_copyright'
Call Stack (most recent call first):
  CMakeLists.txt:25 (ament_copyright)


-- Configuring incomplete, errors occurred!

I thought @tfoote solved that, did you make a pull request for that @tfoote?

@wjwwood
Copy link
Contributor Author

wjwwood commented Mar 31, 2015

The osrf_pycommon package does not pass the syntax check with Python 2.x. Should we remove Python 2.x from the .travis file?

That was a connection timed out error (the pr version of the python 2.7 job passed), I retriggered the job.

We could remove the Python2 check in general though.

@wjwwood
Copy link
Contributor Author

wjwwood commented Mar 31, 2015

CI is passing now.

@wjwwood
Copy link
Contributor Author

wjwwood commented Mar 31, 2015

I'm going to merge this because I tested it locally, it passes the travis CI, and I believe the remaining error on the Windows batch job is unrelated.

wjwwood added a commit that referenced this pull request Mar 31, 2015
updated ament_cmake's on_test for Windows
@wjwwood wjwwood merged commit 49abf07 into master Mar 31, 2015
@wjwwood wjwwood removed the in progress Actively being worked on (Kanban column) label Mar 31, 2015
@wjwwood wjwwood deleted the run_tests_windows branch March 31, 2015 01:17
@wjwwood
Copy link
Contributor Author

wjwwood commented Mar 31, 2015

I changed the job back to not try to change branches.

@dirk-thomas
Copy link
Contributor

How do we want to address the pending comments?

@wjwwood
Copy link
Contributor Author

wjwwood commented Mar 31, 2015

Sorry, I forgot those, I'll update those comments about the wording in a follow on commit. For the one about the --build-tests flag, I'll remove the reference, but we should open another issue if we want to change the flags on the test* verbs.

wjwwood added a commit that referenced this pull request Mar 31, 2015
@wjwwood
Copy link
Contributor Author

wjwwood commented Mar 31, 2015

I addressed the comments as well as I could in 24ddad5.

@dirk-thomas
Copy link
Contributor

Great, thanks.

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

Successfully merging this pull request may close these issues.

ament_tools fails on windows testing
3 participants