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

Allow ignore_marker to be a dangling symlink #344

Merged
merged 2 commits into from
May 4, 2020

Conversation

rotu
Copy link
Contributor

@rotu rotu commented Apr 29, 2020

@dirk-thomas
Copy link
Member

Have you tried running the patch?

@codecov
Copy link

codecov bot commented Apr 29, 2020

Codecov Report

Merging #344 into master will decrease coverage by 0.01%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #344      +/-   ##
==========================================
- Coverage   80.56%   80.54%   -0.02%     
==========================================
  Files          56       56              
  Lines        3237     3239       +2     
  Branches      536      536              
==========================================
+ Hits         2608     2609       +1     
- Misses        592      593       +1     
  Partials       37       37              
Impacted Files Coverage Δ
colcon_core/verb/build.py 0.00% <0.00%> (ø)
colcon_core/package_identification/ignore.py 100.00% <100.00%> (ø)

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 e9e3710...cfdde6b. Read the comment docs.

@rotu
Copy link
Contributor Author

rotu commented Apr 29, 2020

Have you tried running the patch?

I hadn't, as can be seen from CI!

@rotu
Copy link
Contributor Author

rotu commented Apr 30, 2020

This patch now works correctly and is ready for review

@dirk-thomas
Copy link
Member

With the REP update merged (ros-infrastructure/rep#256) I will go ahead ad merge this.

@dirk-thomas dirk-thomas merged commit 66893a8 into colcon:master May 4, 2020
@dirk-thomas dirk-thomas added this to the 0.5.10 milestone May 4, 2020
@chapulina
Copy link

🧑‍🌾

The Appveyor build failed after this PR was merged:

https://ci.appveyor.com/project/dirk-thomas/colcon-core/history

[5.219s] ERROR    colcon.colcon_core.event_reactor:event_reactor.py:86 Exception in event handler extension 'extension3': custom exception
Traceback (most recent call last):
  File "C:\projects\colcon-core\colcon_core\event_reactor.py", line 78, in _notify_observers
    retval = observer(event)
  File "C:\projects\colcon-core\test\test_event_reactor.py", line 43, in __call__
    raise RuntimeError('custom exception')
RuntimeError: custom exception

@dirk-thomas
Copy link
Member

@chapulina The correct error message of the test failure is:

>           assert error.call_count == 2
E           AssertionError: assert 1 == 2
E            +  where 1 = <MagicMock name='error' id='872644937432'>.call_count
test\test_event_reactor.py:80: AssertionError

I retriggered the build and expect it to pass (since it passed before).

@dirk-thomas
Copy link
Member

@chapulina See #345 which makes the test (less | not) flaky.

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

Successfully merging this pull request may close these issues.

None yet

3 participants