ros-tooling / cross_compile

A tool to build ROS and ROS2 workspaces for various targets
Apache License 2.0
188 stars 60 forks source link

Fix and add unit test for incorrect BuildError constructor #107

Closed emersonknapp closed 4 years ago

emersonknapp commented 4 years ago

This issue was revealed by an unrelated network issue in https://github.com/ros-tooling/cross_compile/pull/105/checks?check_run_id=378401015 that caused the docker build to fail.

Fix the issue and add relevant regression tests.

Signed-off-by: Emerson Knapp emerson.b.knapp@gmail.com

codecov[bot] commented 4 years ago

Codecov Report

Merging #107 into master will increase coverage by 4.58%. The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
+ Coverage   69.69%   74.28%   +4.58%     
==========================================
  Files           4        4              
  Lines         264      280      +16     
==========================================
+ Hits          184      208      +24     
+ Misses         80       72       -8
Flag Coverage Δ
#unittests 74.28% <88.88%> (+4.58%) :arrow_up:
Impacted Files Coverage Δ
test/test_sysroot_compiler.py 100% <100%> (ø) :arrow_up:
cross_compile/sysroot_compiler.py 75.36% <50%> (+6.24%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 58a8611...a48ac21. Read the comment docs.

prajakta-gokhale commented 4 years ago

macOS test seems to be failing because of this change.

emersonknapp commented 4 years ago

@prajakta-gokhale yeah. It turns out that Mac has no rosdep rule for docker py, and is quietly grabbing the latest from pip during the pytest step. I am investigating what our options are to solve the discrepancy

emersonknapp commented 4 years ago

To resolve this, I have pinned the dependency version in the setup.py. I believe this is the right move for us as we move to a pure python package anyways, so win-win :) OSX passed, just going to wait on the rest of the checks and will merge if green