Skip to content

Allow ignore_marker to be a dangling symlink#344

Merged
dirk-thomas merged 2 commits intocolcon:masterfrom
rotu:lexists
May 4, 2020
Merged

Allow ignore_marker to be a dangling symlink#344
dirk-thomas merged 2 commits intocolcon:masterfrom
rotu:lexists

Conversation

@rotu
Copy link
Copy Markdown
Contributor

@rotu rotu commented Apr 29, 2020

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Apr 29, 2020

Have you tried running the patch?

@codecov
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Contributor Author

rotu commented Apr 30, 2020

This patch now works correctly and is ready for review

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented May 4, 2020

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
Copy Markdown

chapulina commented May 6, 2020

🧑‍🌾

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
Copy Markdown
Member

dirk-thomas commented May 6, 2020

@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
Copy Markdown
Member

dirk-thomas commented May 6, 2020

@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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

0.5.10

Development

Successfully merging this pull request may close these issues.

3 participants

@rotu @dirk-thomas @chapulina