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

Add initial Travis config #7

Closed
wants to merge 2 commits into
base: releases
from

Conversation

Projects
None yet
2 participants
@paultcochrane
Contributor

paultcochrane commented Feb 9, 2018

This patch adds the ability to build and test Dancer::Session::Cookie on Travis-CI. The reason only Perl 5.22 onwards is used is because there is a dependency on List::Lazy which requires a minimum Perl version of 5.22 (otherwise the build barfs). The dependency installation step also doesn't use the --no-skip-satisfied option to cpanm since IPC::System::Simple doesn't seem to want to build on Travis. Perhaps one could get around that by not running the tests on the dependencies, but I tend not to like doing that, hence the patch's current form.

The fact that List::Lazy requires 5.22+ raises an interesting question: what should the minimum Perl version for this dist? The minimum required Perl version according to syntax is 5.6. Perhaps it's possible to install this module on a 5.6 system, however one can't test it on a 5.6 system. Is it possible (or even sensible) to separate the Perl version restrictions in Dist::Zilla: i.e. have a minimum required Perl version for installation and another for testing?

Also, I'm not 100% certain as to why this PR wants to include the most recent commit on the master branch (3ef091f). I hope this doesn't cause you any problems. If it does cause you problems, please let me know and I'll see if I can fix the issue (I've already tried fixing the issue by using a new branch, but that didn't help...). Also, if you have any questions or comments concerning this PR, please don't hesitate to contact me; if anything needs changing just let me know and I'll update and resubmit as necessary.

yanick and others added some commits Sep 23, 2015

v0.27
  - Remode  'README.pod' from distribution. [GH#6]

  [ STATISTICS ]
    - code churn: 4 files changed, 62 insertions(+), 126 deletions(-)
@yanick

This comment has been minimized.

Contributor

yanick commented Feb 9, 2018

however one can't test it on a 5.6 system.

Sure one can. The dependency to Dist::Zilla and List::Lazy is only for building the distribution. The distribution as it is on CPAN has no such dependency.

I have a few tricks up to sleeve to go around the Travis/dzil problem. I'll try to address this ticket and pull it in over the week-end.

@paultcochrane

This comment has been minimized.

Contributor

paultcochrane commented Feb 9, 2018

So instead of using dzil test, should one use prove instead?

@yanick

This comment has been minimized.

Contributor

yanick commented Feb 10, 2018

So instead of using dzil test, should one use prove instead?

The short answer is: "yup".

The tricky part in the whole thing is that in those Travis tests has a dual role: it's the interpreter for the end-product, and it's the "project compiler" that turns the source code of the repo into CPAN-consumable modules.

One way to get around that would be to only run tests on "built" branches (like releases).

Another way would be to only run the tests on the subset of perls that satisfy both the requirements for the distro itself AND the dist-zilla dance. That one is not very desirable as I'm purposefully on the bleeding edge for my dzil modules, as that would eclipse the actual range of perls covered for the module itself.

And then there is the Occam Razor's way that you found out: just run prove. It won't run the tests on the final, dzil-built modules, nor it will run the tests auto-added by dzil, but that's such a small delta that I don't think it matters much. Plus, if the dzil build was to mess things up, CPANtesters would let us know preeeeeetty soon. :-)

@yanick

This comment has been minimized.

Contributor

yanick commented Feb 10, 2018

@yanick yanick added this to the v0.28 milestone Feb 10, 2018

@yanick yanick closed this Feb 10, 2018

@paultcochrane

This comment has been minimized.

Contributor

paultcochrane commented Feb 11, 2018

Interesting. Why remove the .travis.yml file in the before_install step? Also, why only test from 5.14 onwards? Do the older Perls not work? I think you also need to tell Travis not to build branches which don't have a .travis.yml since the current build (https://www.travis-ci.org/PerlDancer/Dancer-Session-Cookie/builds/339888084) is failing.

@yanick

This comment has been minimized.

Contributor

yanick commented Feb 11, 2018

Why remove the .travis.yml file in the before_install step?

Yeah, it's not really necessary. I think I copied that from a boilerplate template, and the goal is to have the tested code more like the final, non-dev state of the repo. In our case it doesn't make much of a difference, but meh, why not? :-)

Also, why only test from 5.14 onwards? Do the older Perls not work?

Nah, that's me being a jerk. :-) I thought about how far I'm willing to go for backward compatibility, and I've decided that 5 major releases of Perl is a decent line in the sand. Mind you, the module can still work for older releases than that. I just won't pour a lot of effort into it.

I think you also need to tell Travis not to build branches which don't have a .travis.yml since the current build (https://www.travis-ci.org/PerlDancer/Dancer-Session-Cookie/builds/339888084) is failing.

Yup, already did. :-)

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