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

[Windows] Add Azure CI file and fixing test breaks on Windows #661

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

seanyen
Copy link
Contributor

@seanyen seanyen commented Feb 15, 2019

This change is to add Azure DevOps pipelines for running Windows CI, and also fixes test breaks on Windows:

Highlight the changes on the production code side:

  • src/rosdep2/platforms/source.py
    • add missing tarfile.close()
  • src/rosdep2/shell_utils.py
    • add script suffix .bat on Windows to make the shell able to run the script.
  • src/rosdep2/main.py
    • fix missing os.EX_USAGE on Windows causing runtime errors.
    • use portable os.pathsep to replace :
  • src/rosdep2/platforms/windows.py & src/rosdep2/__init__.py
    • Add stub rosdep2.platforms.windows to avoid Unsupported OS error on Windows.
  • src/rosdep2/installers.py
    • conditionally add 'sudo -H' based on availability of os.geteuid().

Summary of changes on the test code side:

  • Fix test cases to accommodate the sudo -H changes.
  • Normalize file:// URIs in test cases.
  • Conditionally generate test script for bash scripts and batch scripts.
  • Add missing mock function for test_RosdepInstaller_install_resolved
  • Add alternative implementation for os.path.samefile for Python 2 on Windows.

@seanyen
Copy link
Contributor Author

seanyen commented Feb 15, 2019

Azure CI result is published here: https://ros-win.visualstudio.com/ros-win/_build/results?buildId=3150

@codecov-io
Copy link

codecov-io commented Feb 15, 2019

Codecov Report

Attention: Patch coverage is 93.33333% with 3 lines in your changes missing coverage. Please review.

Project coverage is 76.51%. Comparing base (f771260) to head (1e066a5).
Report is 113 commits behind head on master.

Files with missing lines Patch % Lines
src/rosdep2/shell_utils.py 92.59% 2 Missing ⚠️
src/rosdep2/main.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #661      +/-   ##
==========================================
+ Coverage   75.99%   76.51%   +0.51%     
==========================================
  Files          40       41       +1     
  Lines        3129     3330     +201     
==========================================
+ Hits         2378     2548     +170     
- Misses        751      782      +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@seanyen
Copy link
Contributor Author

seanyen commented Apr 22, 2019

@nuclearsandwich Just a friendly ping. This is a PR focused on fixing test failures for rosdep on Windows. Let me know anything to iterate.

Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Overall this looks good to me. Though I saw that there were some places to reduce the size of the diff/additional code for better long term maintenance.

I'm a little surprised that the sudo commands were necessary to adjust for windows as in the unit tests I expect them to not be executed and just be string matching, but I think this is an improvement so we might as well use it.

test/test_rosdep_source.py Outdated Show resolved Hide resolved
test/test_rosdep_installers.py Outdated Show resolved Hide resolved
@seanyen seanyen force-pushed the windows_port_test_fix_and_ci branch 2 times, most recently from 985b6f6 to 4b15bc3 Compare June 3, 2019 20:21
@seanyen
Copy link
Contributor Author

seanyen commented Jun 3, 2019

@tfoote @nuclearsandwich I pushed new changes to address the duplicate logic feedbacks and rebase the code to the latest master. And the results are green for Windows CI: https://ros-win.visualstudio.com/ros-win/_build/results?buildId=4336

Hope you can take a look for the new iteration! Thanks!

@seanyen seanyen force-pushed the windows_port_test_fix_and_ci branch from 6be5f9e to 6c2072e Compare June 4, 2019 18:10
@seanyen
Copy link
Contributor Author

seanyen commented Aug 13, 2019

@cottsay @tfoote @nuclearsandwich Please excuse me to ping you all. Hope this is still under the radar and it would be nice to see more Windows support for rosdep.

@seanyen
Copy link
Contributor Author

seanyen commented Nov 18, 2019

travis seemed to fail on a infrastructure issue:

Homebrew must be run under Ruby 2.6! You're running 2.3.3. 

@nuclearsandwich
Copy link
Contributor

@seanyen now that #751 is finally merged would you like to rework this PR to add Windows CI via GitHub Action?

Tobias-Fischer added a commit to RoboStack/rosdep that referenced this pull request May 5, 2021
Tobias-Fischer added a commit to RoboStack/rosdep that referenced this pull request May 5, 2021
Tobias-Fischer added a commit to RoboStack/rosdep that referenced this pull request May 5, 2021
Tobias-Fischer added a commit to RoboStack/rosdep that referenced this pull request May 5, 2021
Tobias-Fischer added a commit to RoboStack/rosdep that referenced this pull request May 12, 2021
@Tobias-Fischer Tobias-Fischer mentioned this pull request May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants