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

fix tests failing on cpan testers reports #44

Closed
wants to merge 1 commit into
base: dev
from

Conversation

Projects
None yet
2 participants
@lharey

lharey commented Aug 10, 2015

Hi

As per my email re the cpan pull request challenge.

This should hopefully fix the failing tests on cpan testers - http://www.cpantesters.org/distro/H/HTML-Lint.html

example fail -
http://www.cpantesters.org/cpan/report/a38f9f44-3d6f-11e5-a615-fd3ac5fc1a82

Cheers
Lisa

@petdance

This comment has been minimized.

Show comment
Hide comment
@petdance

petdance Aug 10, 2015

Owner

What is the bug in the original code that this fixes? It's not clear to me from the patch.

Owner

petdance commented Aug 10, 2015

What is the bug in the original code that this fixes? It's not clear to me from the patch.

@lharey

This comment has been minimized.

Show comment
Hide comment
@lharey

lharey Aug 10, 2015

Hi

The t/12-html-fragment_ok.t seems to be failing - see
http://www.cpantesters.org/cpan/report/a38f9f44-3d6f-11e5-a615-fd3ac5fc1a82

I think its a bug in the test and not the actual module.

LintTest.pl in that it expects the order of the arrays @expected and
@linesets to always match.

I've amended the test so that it doesn't rely on the order of the arrays to
be the same

Does this seem like the right thing to do?

Regards
Lisa

On 10 August 2015 at 15:01, Andy Lester notifications@github.com wrote:

What is the bug in the original code that this fixes? It's not clear to me
from the patch.


Reply to this email directly or view it on GitHub
#44 (comment).

lharey commented Aug 10, 2015

Hi

The t/12-html-fragment_ok.t seems to be failing - see
http://www.cpantesters.org/cpan/report/a38f9f44-3d6f-11e5-a615-fd3ac5fc1a82

I think its a bug in the test and not the actual module.

LintTest.pl in that it expects the order of the arrays @expected and
@linesets to always match.

I've amended the test so that it doesn't rely on the order of the arrays to
be the same

Does this seem like the right thing to do?

Regards
Lisa

On 10 August 2015 at 15:01, Andy Lester notifications@github.com wrote:

What is the bug in the original code that this fixes? It's not clear to me
from the patch.


Reply to this email directly or view it on GitHub
#44 (comment).

@petdance

This comment has been minimized.

Show comment
Hide comment
@petdance

petdance Aug 10, 2015

Owner

I'm wondering why we're even comparing the array ourselves when things like is_deeply() exist. I would think we could do something like:

is_deeply( [sort @errors], [sort @expected] );
Owner

petdance commented Aug 10, 2015

I'm wondering why we're even comparing the array ourselves when things like is_deeply() exist. I would think we could do something like:

is_deeply( [sort @errors], [sort @expected] );
@lharey

This comment has been minimized.

Show comment
Hide comment
@lharey

lharey Aug 11, 2015

Hi

I've had a look to see if thats possible, but the two arrays are not in the
same formats. @errors are objects and @expected can also include regular
expressions.

Regards
Lisa

On 10 August 2015 at 15:31, Andy Lester notifications@github.com wrote:

I'm wondering why we're even comparing the array ourselves when things
like is_deeply() exist. I would think we could do something like:

is_deeply( [sort @errors], [sort @expected] );


Reply to this email directly or view it on GitHub
#44 (comment).

lharey commented Aug 11, 2015

Hi

I've had a look to see if thats possible, but the two arrays are not in the
same formats. @errors are objects and @expected can also include regular
expressions.

Regards
Lisa

On 10 August 2015 at 15:31, Andy Lester notifications@github.com wrote:

I'm wondering why we're even comparing the array ourselves when things
like is_deeply() exist. I would think we could do something like:

is_deeply( [sort @errors], [sort @expected] );


Reply to this email directly or view it on GitHub
#44 (comment).

@petdance

This comment has been minimized.

Show comment
Hide comment
@petdance

petdance Dec 7, 2016

Owner

This has been fixed by another pull request that I got to first. Thanks for your help. See 0389052.

Owner

petdance commented Dec 7, 2016

This has been fixed by another pull request that I got to first. Thanks for your help. See 0389052.

@petdance petdance closed this Dec 7, 2016

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