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

tests depend on Test::SharedFork explicitly #64

Merged
merged 1 commit into from Apr 6, 2015

Conversation

Projects
None yet
2 participants
@bjakubski
Contributor

bjakubski commented Apr 5, 2015

It seems that tests require Test::SharedFork to work, but it is not explicitly declared. Everything worked, because Test::TCP requires Test::SharedFork, and Test::TCP is an official dependency.

@lestrrat

This comment has been minimized.

Show comment
Hide comment
@lestrrat

lestrrat Apr 5, 2015

Collaborator

Yes, that was exactly my intention (not mentioning Test::SharedFork because Test::TCP already requires it). Did it cause problems?

Collaborator

lestrrat commented Apr 5, 2015

Yes, that was exactly my intention (not mentioning Test::SharedFork because Test::TCP already requires it). Did it cause problems?

@bjakubski

This comment has been minimized.

Show comment
Hide comment
@bjakubski

bjakubski Apr 6, 2015

Contributor

No, and it won't unless Test::TCP will stop (for any reason) requiring
SharedFork.
I think it is sensible to explicitly require direct dependencies so we are
immune from changes in other distributions

On Apr 6, 2015 12:04 AM, "lestrrat" notifications@github.com wrote:

Yes, that was exactly my intention (not mentioning Test::SharedFork
because Test::TCP already requires it). Did it cause problems?


Reply to this email directly or view it on GitHub.

Contributor

bjakubski commented Apr 6, 2015

No, and it won't unless Test::TCP will stop (for any reason) requiring
SharedFork.
I think it is sensible to explicitly require direct dependencies so we are
immune from changes in other distributions

On Apr 6, 2015 12:04 AM, "lestrrat" notifications@github.com wrote:

Yes, that was exactly my intention (not mentioning Test::SharedFork
because Test::TCP already requires it). Did it cause problems?


Reply to this email directly or view it on GitHub.

@lestrrat

This comment has been minimized.

Show comment
Hide comment
@lestrrat

lestrrat Apr 6, 2015

Collaborator

Fair enough. Was just wondering why all of the sudden I got a PR for this.

Collaborator

lestrrat commented Apr 6, 2015

Fair enough. Was just wondering why all of the sudden I got a PR for this.

@lestrrat

This comment has been minimized.

Show comment
Hide comment
@lestrrat

lestrrat Apr 6, 2015

Collaborator

And this test failure is annoying... but merging anyway

Collaborator

lestrrat commented Apr 6, 2015

And this test failure is annoying... but merging anyway

lestrrat added a commit that referenced this pull request Apr 6, 2015

Merge pull request #64 from bjakubski/master
tests depend on Test::SharedFork explicitly

@lestrrat lestrrat merged commit 221e836 into lestrrat-p5:master Apr 6, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@bjakubski

This comment has been minimized.

Show comment
Hide comment
@bjakubski

bjakubski Apr 6, 2015

Contributor

If you don't mind waiting some time then I'm willing to have a good look at those failures in following days (ATM I'm having extremely spotty net connection - just getting into travis logs took me 15 minutes...).

This PR comes from CPAN Pull Request challenge - I got LibZMQ2 dist assigned for april. For me this challenge it is an opportunity to make a habit out of github contributions and learn a thing or two about CPAN ecosystem in the process. I hope that my changes are beneficial to distributions they are against, but I can always be wrong or just not align with author wishes :-).

If you find change wrong or just controversial then feel free to reject such PR (or in this case revert), I'll be absolutely fine with that!

Contributor

bjakubski commented Apr 6, 2015

If you don't mind waiting some time then I'm willing to have a good look at those failures in following days (ATM I'm having extremely spotty net connection - just getting into travis logs took me 15 minutes...).

This PR comes from CPAN Pull Request challenge - I got LibZMQ2 dist assigned for april. For me this challenge it is an opportunity to make a habit out of github contributions and learn a thing or two about CPAN ecosystem in the process. I hope that my changes are beneficial to distributions they are against, but I can always be wrong or just not align with author wishes :-).

If you find change wrong or just controversial then feel free to reject such PR (or in this case revert), I'll be absolutely fine with that!

@lestrrat

This comment has been minimized.

Show comment
Hide comment
@lestrrat

lestrrat Apr 6, 2015

Collaborator

Well you're welcome to look at it. I suspect some breaking change on libczmq1 (the C library) but I haven't looked far enough to make any finer grained guesses.

Also, this repo is a bit peculiar for a Perl module as it contains many bindings all in one... beware of that fact when you look into other modules in the future ;)

Collaborator

lestrrat commented Apr 6, 2015

Well you're welcome to look at it. I suspect some breaking change on libczmq1 (the C library) but I haven't looked far enough to make any finer grained guesses.

Also, this repo is a bit peculiar for a Perl module as it contains many bindings all in one... beware of that fact when you look into other modules in the future ;)

@bjakubski

This comment has been minimized.

Show comment
Hide comment
@bjakubski

bjakubski Apr 6, 2015

Contributor

Failing tests are a simple issue of new Test::TCP release not declaring its runtime dependencies correctly - IO::Socket::IP is needed on runtime, but declared as test-only dependency. In Travis run for ZMQ Test::TCP gets installed, but it does not pull IO::Socket::IP(due to --notest). So ZMQ tests have Test::TCP available, but it fails during 'use'.

I've created tokuhirom/Test-TCP#36 and if it gets released it should fix ZMQ travis failures.

Contributor

bjakubski commented Apr 6, 2015

Failing tests are a simple issue of new Test::TCP release not declaring its runtime dependencies correctly - IO::Socket::IP is needed on runtime, but declared as test-only dependency. In Travis run for ZMQ Test::TCP gets installed, but it does not pull IO::Socket::IP(due to --notest). So ZMQ tests have Test::TCP available, but it fails during 'use'.

I've created tokuhirom/Test-TCP#36 and if it gets released it should fix ZMQ travis failures.

@lestrrat

This comment has been minimized.

Show comment
Hide comment
@lestrrat

lestrrat Apr 7, 2015

Collaborator

Ah, was it? okay that works! Thank you :)

Collaborator

lestrrat commented Apr 7, 2015

Ah, was it? okay that works! Thank you :)

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