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

Adds coverage tests #51

Merged
merged 2 commits into from Jun 4, 2017

Conversation

Projects
None yet
4 participants
@JJ
Contributor

JJ commented May 28, 2017

Following the example of @moose, but it's pretty much the same you pointed me to. For the time being, it prints coverage in the logs (spoiler: not good) but I guess that if it finds a matching repo in coveralls.io, which you said you had set up, it should upload it there. Tell me if it does not work

@JJ

This comment has been minimized.

Show comment
Hide comment
@JJ

JJ May 28, 2017

Contributor

Hey, who would've said that. It works. I love it when that happens.

Contributor

JJ commented May 28, 2017

Hey, who would've said that. It works. I love it when that happens.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 28, 2017

Coverage Status

Changes Unknown when pulling 27cfeda on JJ:master into ** on libwww-perl:master**.

coveralls commented May 28, 2017

Coverage Status

Changes Unknown when pulling 27cfeda on JJ:master into ** on libwww-perl:master**.

@karenetheridge

What is the purpose of this commit?

@karenetheridge

As long as we support these perl versions we should continue to test them on travis.

@JJ

This comment has been minimized.

Show comment
Hide comment
@JJ

JJ May 28, 2017

Contributor

Address issue #50 . This is the suggestion I received.

Contributor

JJ commented May 28, 2017

Address issue #50 . This is the suggestion I received.

@JJ

This comment has been minimized.

Show comment
Hide comment
@JJ

JJ May 28, 2017

Contributor
Contributor

JJ commented May 28, 2017

@karenetheridge

This comment has been minimized.

Show comment
Hide comment
@karenetheridge

karenetheridge May 28, 2017

Member
Member

karenetheridge commented May 28, 2017

@JJ

This comment has been minimized.

Show comment
Hide comment
@JJ

JJ May 28, 2017

Contributor
Contributor

JJ commented May 28, 2017

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 28, 2017

Coverage Status

Changes Unknown when pulling b462f10 on JJ:master into ** on libwww-perl:master**.

coveralls commented May 28, 2017

Coverage Status

Changes Unknown when pulling b462f10 on JJ:master into ** on libwww-perl:master**.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 28, 2017

Coverage Status

Changes Unknown when pulling 2fb4000 on JJ:master into ** on libwww-perl:master**.

coveralls commented May 28, 2017

Coverage Status

Changes Unknown when pulling 2fb4000 on JJ:master into ** on libwww-perl:master**.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 28, 2017

Coverage Status

Changes Unknown when pulling 087acdb on JJ:master into ** on libwww-perl:master**.

coveralls commented May 28, 2017

Coverage Status

Changes Unknown when pulling 087acdb on JJ:master into ** on libwww-perl:master**.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 28, 2017

Coverage Status

Changes Unknown when pulling 087acdb on JJ:master into ** on libwww-perl:master**.

coveralls commented May 28, 2017

Coverage Status

Changes Unknown when pulling 087acdb on JJ:master into ** on libwww-perl:master**.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 28, 2017

Coverage Status

Changes Unknown when pulling 087acdb on JJ:master into ** on libwww-perl:master**.

coveralls commented May 28, 2017

Coverage Status

Changes Unknown when pulling 087acdb on JJ:master into ** on libwww-perl:master**.

@JJ

This comment has been minimized.

Show comment
Hide comment
@JJ

JJ May 28, 2017

Contributor

It's testing OK now. I only had to change the copyright year in dist.ini. The gist of it is that there's some rule in dist.ini that makes it check that, and that using build-dist was triggering installation of Dist::Zilla and rules, while the previous Travis CI wasn't doing that. Boils down to: it's got coverage testing now, and it also tests for all versions of perl previously used.

Contributor

JJ commented May 28, 2017

It's testing OK now. I only had to change the copyright year in dist.ini. The gist of it is that there's some rule in dist.ini that makes it check that, and that using build-dist was triggering installation of Dist::Zilla and rules, while the previous Travis CI wasn't doing that. Boils down to: it's got coverage testing now, and it also tests for all versions of perl previously used.

@oalders

Thanks for this. I just wanted to see if we could golf down the Travis config a touch more. Other than that, could you squash your commit history?

Show outdated Hide outdated .travis.yml
- perl: 5.22
env: COVERAGE=1 # enables coverage+coveralls reporting
include:
- perl: 5.24

This comment has been minimized.

@oalders

oalders Jun 1, 2017

Member

Why do we need coverage on multiple Perls?

@oalders

oalders Jun 1, 2017

Member

Why do we need coverage on multiple Perls?

Show outdated Hide outdated .travis.yml
- build-dist
- cd $BUILD_DIR # $BUILD_DIR is set by the build-dist command
install:
- cpan-install ExtUtils::MakeMaker~6.68 --deps

This comment has been minimized.

@oalders

oalders Jun 1, 2017

Member

Why do we need this specific version of ExtUtils::MakeMaker? I just see a comment about cargo culting, but it's not clear to me from where or why.

@oalders

oalders Jun 1, 2017

Member

Why do we need this specific version of ExtUtils::MakeMaker? I just see a comment about cargo culting, but it's not clear to me from where or why.

script:
- prove -l -j$(test-jobs) $(test-files) # parallel testing
after_success:
- coverage-report

This comment has been minimized.

@oalders

oalders Jun 1, 2017

Member

It looks to me like most of these are just the default perl-helpers behaviour. Have you tried seeing what happens if we delete everything that aligns with the defaults and try:

before_install:
  - eval $(curl https://travis-perl.github.io/init) --auto

?

See https://github.com/travis-perl/helpers

@oalders

oalders Jun 1, 2017

Member

It looks to me like most of these are just the default perl-helpers behaviour. Have you tried seeing what happens if we delete everything that aligns with the defaults and try:

before_install:
  - eval $(curl https://travis-perl.github.io/init) --auto

?

See https://github.com/travis-perl/helpers

This comment has been minimized.

@JJ

JJ Jun 3, 2017

Contributor

Better not to. Took me a while to debug the file. What that file does is to create a helper dir and update it. Not too fond of these shell scripts. Anyway, we need prove and we need coverage report, so I will leave these one as they are.

@JJ

JJ Jun 3, 2017

Contributor

Better not to. Took me a while to debug the file. What that file does is to create a helper dir and update it. Not too fond of these shell scripts. Anyway, we need prove and we need coverage report, so I will leave these one as they are.

Getting the coverage tests from @moose
You need to install dependencies anyway

Eliminates errored versions in deps

Eliminates old versions

Which fail in the coverage test

Adds coverage for the other working version.

I would say this closes #50, but feel free to open it back if it does
not work.

Test with conditional build

Testing conditional execution of things

Once again debugging (c) year

Well, I'd be damned

It was an error in the `dist.ini` file that, for some reason, yielded
a problem only in versions older than 5.22. Those versions do not seem
to admit a copyright string this way: "whatever-whateverelse". I have
changed it to this year, which I guess is no big deal, and it seems to
be working now, at least for the test I have been running.

Since 5.22 and 5.24 are tested for coverage, it's probably reasonable
to not test them again. I can change that if needed. And this, yes,
closes  #50

Eliminates coverage in multiple Perls

@oalders a priori, different versions of Perl could take different
branches and thus have a different coverage. In that case, it would be
probably more reasonable that the versions should be as far as
possible. As reasonable as having coverage for only one version, like
here.

Changing to generic version
@JJ

This comment has been minimized.

Show comment
Hide comment
@JJ

JJ Jun 3, 2017

Contributor

Squashed into two commits, most of the things in the second one. Changes requested done.

Contributor

JJ commented Jun 3, 2017

Squashed into two commits, most of the things in the second one. Changes requested done.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jun 3, 2017

Coverage Status

Changes Unknown when pulling 7c2b876 on JJ:master into ** on libwww-perl:master**.

coveralls commented Jun 3, 2017

Coverage Status

Changes Unknown when pulling 7c2b876 on JJ:master into ** on libwww-perl:master**.

@oalders

oalders approved these changes Jun 4, 2017

@oalders

This comment has been minimized.

Show comment
Hide comment
@oalders

oalders Jun 4, 2017

Member

Thanks very much @JJ!

Member

oalders commented Jun 4, 2017

Thanks very much @JJ!

@oalders oalders merged commit 14df39e into libwww-perl:master Jun 4, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 77.419%
Details
@JJ

This comment has been minimized.

Show comment
Hide comment
@JJ

JJ Jun 4, 2017

Contributor
Contributor

JJ commented Jun 4, 2017

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