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

Allow -u|--unsorted and -a|--accelerated in any order in new() #7

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@pauloscustodio
Contributor

pauloscustodio commented Jun 7, 2015

The code for List::Compare::new() only detected correctly the unsorted and
accelerated options if they were given in this exact order.

This not-intuitive behavior was described in a documentation note in the POD.

Moreover the code to detect -u|--unsorted was repeated in three locations:
List::Compare::new(), List::Compare::Base::_Auxiliary::_alt_construct_tester() and
List::Compare::Base::_Auxiliary::_alt_construct_tester_3().

Factored the code to detect -u|--unsorted and/or -a|--accelerated in any order in
List::Compare::Base::_Auxiliary::_parse_optios(), and called this new function from the
three places where the parsing was being done.

Changed t/04_oo_lists_dual_acc_unsorted.t to try all possible combinations of short/long
options, in any order, with presence/absence of each option and check the result of the
object construction to make sure the options were being take into account.

Removed the restriction of the options mandatory order from the POD.

Allow -u|--unsorted and -a|--accelerated in any order in new()
The code for List::Compare::new() only detected correctly the unsorted and
accelerated options if they were given in this exact order.

This not-intuitive behaviour was described in a documentation note in the POD.

Moreover the code to detect -u|--unsorted was repeated in three locations:
List::Compare::new(), List::Compare::Base::_Auxiliary::_alt_construct_tester() and
List::Compare::Base::_Auxiliary::_alt_construct_tester_3().

Factored the code to detect -u|--unsorted and/or -a|--accelerated in any order in
List::Compare::Base::_Auxiliary::_parse_optios(), and called this new function from the
three places where the parsing was being done.

Changed t/04_oo_lists_dual_acc_unsorted.t to try all possible combinations of short/long
options, in any order, with presence/absence of each option and check the result of the
object construction to make sure the options were being take into account.

Removed the restriction of the options mandatory order from the POD.
@jkeenan

This comment has been minimized.

Show comment
Hide comment
@jkeenan

jkeenan Jun 7, 2015

Owner

Thanks. I probably won't get a chance to look at this until after YAPC::NA.

Owner

jkeenan commented Jun 7, 2015

Thanks. I probably won't get a chance to look at this until after YAPC::NA.

pauloscustodio added some commits Jun 10, 2015

Test all combinations of constructor with list and hash ref arguments
Add tests to call constructor with all option combinations in a hash-ref argument.
Fix indentation
Left "wrong" indentation in the previuous commit to better show the differences added.
@jkeenan

This comment has been minimized.

Show comment
Hide comment
@jkeenan

jkeenan Jun 20, 2015

Owner

I believe you have suggested a worthwhile feature improvement. Just reading the title of this Issue gave me enough of a hint to go to places in the source code that currently mandate that -u or --unsorted be passed in before -a or --accelerated. And, at least at first glance, internal subroutine _parse_args() looks good.

However, given that List-Compare's test suite has for 13 years been composed of explicit unit tests, I'm reluctant to accept your refactoring of the tests in the files in the pull request into nested foreach loops. At $job one of my predecessor's was, in the name of "efficiency", in the habit of nesting tests in foreach loops as many as 10 deep -- and then he had to special-case all kinds of code with 'if' blocks deep inside the inner loops. It became impossible to figure out where one was in the file's code when a failure occurred. Some of these tests have defied refactoring for 3-1/2 years!

If you'd be willing to rework this by following the style of the existing test files, I'd be willing to take another look.

Thank you very much.
Jim Keenan

Owner

jkeenan commented Jun 20, 2015

I believe you have suggested a worthwhile feature improvement. Just reading the title of this Issue gave me enough of a hint to go to places in the source code that currently mandate that -u or --unsorted be passed in before -a or --accelerated. And, at least at first glance, internal subroutine _parse_args() looks good.

However, given that List-Compare's test suite has for 13 years been composed of explicit unit tests, I'm reluctant to accept your refactoring of the tests in the files in the pull request into nested foreach loops. At $job one of my predecessor's was, in the name of "efficiency", in the habit of nesting tests in foreach loops as many as 10 deep -- and then he had to special-case all kinds of code with 'if' blocks deep inside the inner loops. It became impossible to figure out where one was in the file's code when a failure occurred. Some of these tests have defied refactoring for 3-1/2 years!

If you'd be willing to rework this by following the style of the existing test files, I'd be willing to take another look.

Thank you very much.
Jim Keenan

Unrolled loops in test for all options combinations
Replaced for-loops that were testing all the combinations of options
-u, --unsorted, -a, --accelerated by the repeated sequence of test code.
@pauloscustodio

This comment has been minimized.

Show comment
Hide comment
@pauloscustodio

pauloscustodio Jul 2, 2015

Contributor

Done as suggested.

Contributor

pauloscustodio commented Jul 2, 2015

Done as suggested.

@jkeenan

This comment has been minimized.

Show comment
Hide comment
@jkeenan

jkeenan Jul 3, 2015

Owner

I pulled down a patch version of your pull request for local inspection. I squashed your commits into one, then substituted 4 spaces for your hard tabs. I then went to 'make test'. I got failures in one file.

$ prove -vb t/04_oo_lists_dual_acc_unsorted.t 
t/04_oo_lists_dual_acc_unsorted.t .. 
1..170
ok 1 - constructor returned true value
ok 2 - unsorted:  got expected union
...
ok 119 - Constructor worked with '-a' options
ok 120 - Results not pre-computed
ok 121 - Results ok
ok 122 - Results not stored
ok 123 - Constructor worked with '--unsorted' options
not ok 124 - Results pre-computed
not ok 125 - Results ok

#   Failed test 'Results pre-computed'
#   at t/04_oo_lists_dual_acc_unsorted.t line 510.
#     Structures begin differing at:
#          $got->[1] = 'edward'
#     $expected->[1] = 'delta'

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 511.
#     Structures begin differing at:
#          $got->[1] = 'edward'
#     $expected->[1] = 'delta'
ok 126 - Constructor worked with '--unsorted' options
not ok 127 - Results pre-computed

#   Failed test 'Results pre-computed'
#   at t/04_oo_lists_dual_acc_unsorted.t line 520.
#     Structures begin differing at:
#          $got->[0] = 'delta'
#     $expected->[0] = 'golfer'
not ok 128 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 521.
#     Structures begin differing at:
#          $got->[0] = 'delta'
#     $expected->[0] = 'golfer'
ok 129 - Constructor worked with '-u' options
not ok 130 - Results pre-computed

#   Failed test 'Results pre-computed'
#   at t/04_oo_lists_dual_acc_unsorted.t line 530.
#     Structures begin differing at:
#          $got->[1] = 'baker'
#     $expected->[1] = 'delta'
not ok 131 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 531.
#     Structures begin differing at:
#          $got->[1] = 'baker'
#     $expected->[1] = 'delta'
ok 132 - Constructor worked with '-u' options
not ok 133 - Results pre-computed

#   Failed test 'Results pre-computed'
#   at t/04_oo_lists_dual_acc_unsorted.t line 540.
#     Structures begin differing at:
#          $got->[0] = 'baker'
#     $expected->[0] = 'golfer'
not ok 134 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 541.
#     Structures begin differing at:
#          $got->[0] = 'baker'
#     $expected->[0] = 'golfer'
ok 135 - Constructor worked with '--accelerated --unsorted' options
ok 136 - Results not pre-computed
not ok 137 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 551.
#     Structures begin differing at:
#          $got->[0] = 'camera'
#     $expected->[0] = 'golfer'
ok 138 - Results not stored
ok 139 - Constructor worked with '--accelerated --unsorted' options
ok 140 - Results not pre-computed
not ok 141 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 562.
#     Structures begin differing at:
#          $got->[0] = 'delta'
#     $expected->[0] = 'golfer'
ok 142 - Results not stored
ok 143 - Constructor worked with '--accelerated -u' options
ok 144 - Results not pre-computed
not ok 145 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 572.
#     Structures begin differing at:
#          $got->[1] = 'baker'
#     $expected->[1] = 'delta'
ok 146 - Results not stored
ok 147 - Constructor worked with '--unsorted --accelerated' options
ok 148 - Results not pre-computed
not ok 149 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 582.
#     Structures begin differing at:
#          $got->[0] = 'delta'
#     $expected->[0] = 'golfer'
ok 150 - Results not stored
ok 151 - Constructor worked with '--unsorted -a' options
ok 152 - Results not pre-computed
not ok 153 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 592.
#     Structures begin differing at:
#          $got->[1] = 'fargo'
#     $expected->[1] = 'delta'
ok 154 - Results not stored
ok 155 - Constructor worked with '-a --unsorted' options
ok 156 - Results not pre-computed
not ok 157 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 602.
#     Structures begin differing at:
#          $got->[3] = 'fargo'
#     $expected->[3] = 'baker'
ok 158 - Results not stored
ok 159 - Constructor worked with '-a -u' options
ok 160 - Results not pre-computed
not ok 161 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 612.
#     Structures begin differing at:
#          $got->[2] = 'fargo'
#     $expected->[2] = 'edward'
ok 162 - Results not stored
ok 163 - Constructor worked with '-u --accelerated' options
ok 164 - Results not pre-computed
not ok 165 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 622.
#     Structures begin differing at:
#          $got->[0] = 'baker'
#     $expected->[0] = 'golfer'
ok 166 - Results not stored
ok 167 - Constructor worked with '-u -a' options
ok 168 - Results not pre-computed
not ok 169 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 632.
#     Structures begin differing at:
#          $got->[0] = 'baker'
#     $expected->[0] = 'golfer'
ok 170 - Results not stored
# Looks like you failed 17 tests of 170.
Dubious, test returned 17 (wstat 4352, 0x1100)
Failed 17/170 subtests 

Test Summary Report
-------------------
t/04_oo_lists_dual_acc_unsorted.t (Wstat: 4352 Tests: 170 Failed: 17)
  Failed tests:  124-125, 127-128, 130-131, 133-134, 137
                141, 145, 149, 153, 157, 161, 165, 169
  Non-zero exit status: 17
Files=1, Tests=170,  0 wallclock secs ( 0.02 usr  0.01 sys +  0.04 cusr  0.01 csys =  0.08 CPU)
Result: FAIL

I pushed the branch I was working in to my github:
https://github.com/jkeenan/list-compare/tree/paulo-20150702

Can you investigate?

Thank you very much.
Jim Keenan

Owner

jkeenan commented Jul 3, 2015

I pulled down a patch version of your pull request for local inspection. I squashed your commits into one, then substituted 4 spaces for your hard tabs. I then went to 'make test'. I got failures in one file.

$ prove -vb t/04_oo_lists_dual_acc_unsorted.t 
t/04_oo_lists_dual_acc_unsorted.t .. 
1..170
ok 1 - constructor returned true value
ok 2 - unsorted:  got expected union
...
ok 119 - Constructor worked with '-a' options
ok 120 - Results not pre-computed
ok 121 - Results ok
ok 122 - Results not stored
ok 123 - Constructor worked with '--unsorted' options
not ok 124 - Results pre-computed
not ok 125 - Results ok

#   Failed test 'Results pre-computed'
#   at t/04_oo_lists_dual_acc_unsorted.t line 510.
#     Structures begin differing at:
#          $got->[1] = 'edward'
#     $expected->[1] = 'delta'

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 511.
#     Structures begin differing at:
#          $got->[1] = 'edward'
#     $expected->[1] = 'delta'
ok 126 - Constructor worked with '--unsorted' options
not ok 127 - Results pre-computed

#   Failed test 'Results pre-computed'
#   at t/04_oo_lists_dual_acc_unsorted.t line 520.
#     Structures begin differing at:
#          $got->[0] = 'delta'
#     $expected->[0] = 'golfer'
not ok 128 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 521.
#     Structures begin differing at:
#          $got->[0] = 'delta'
#     $expected->[0] = 'golfer'
ok 129 - Constructor worked with '-u' options
not ok 130 - Results pre-computed

#   Failed test 'Results pre-computed'
#   at t/04_oo_lists_dual_acc_unsorted.t line 530.
#     Structures begin differing at:
#          $got->[1] = 'baker'
#     $expected->[1] = 'delta'
not ok 131 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 531.
#     Structures begin differing at:
#          $got->[1] = 'baker'
#     $expected->[1] = 'delta'
ok 132 - Constructor worked with '-u' options
not ok 133 - Results pre-computed

#   Failed test 'Results pre-computed'
#   at t/04_oo_lists_dual_acc_unsorted.t line 540.
#     Structures begin differing at:
#          $got->[0] = 'baker'
#     $expected->[0] = 'golfer'
not ok 134 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 541.
#     Structures begin differing at:
#          $got->[0] = 'baker'
#     $expected->[0] = 'golfer'
ok 135 - Constructor worked with '--accelerated --unsorted' options
ok 136 - Results not pre-computed
not ok 137 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 551.
#     Structures begin differing at:
#          $got->[0] = 'camera'
#     $expected->[0] = 'golfer'
ok 138 - Results not stored
ok 139 - Constructor worked with '--accelerated --unsorted' options
ok 140 - Results not pre-computed
not ok 141 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 562.
#     Structures begin differing at:
#          $got->[0] = 'delta'
#     $expected->[0] = 'golfer'
ok 142 - Results not stored
ok 143 - Constructor worked with '--accelerated -u' options
ok 144 - Results not pre-computed
not ok 145 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 572.
#     Structures begin differing at:
#          $got->[1] = 'baker'
#     $expected->[1] = 'delta'
ok 146 - Results not stored
ok 147 - Constructor worked with '--unsorted --accelerated' options
ok 148 - Results not pre-computed
not ok 149 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 582.
#     Structures begin differing at:
#          $got->[0] = 'delta'
#     $expected->[0] = 'golfer'
ok 150 - Results not stored
ok 151 - Constructor worked with '--unsorted -a' options
ok 152 - Results not pre-computed
not ok 153 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 592.
#     Structures begin differing at:
#          $got->[1] = 'fargo'
#     $expected->[1] = 'delta'
ok 154 - Results not stored
ok 155 - Constructor worked with '-a --unsorted' options
ok 156 - Results not pre-computed
not ok 157 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 602.
#     Structures begin differing at:
#          $got->[3] = 'fargo'
#     $expected->[3] = 'baker'
ok 158 - Results not stored
ok 159 - Constructor worked with '-a -u' options
ok 160 - Results not pre-computed
not ok 161 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 612.
#     Structures begin differing at:
#          $got->[2] = 'fargo'
#     $expected->[2] = 'edward'
ok 162 - Results not stored
ok 163 - Constructor worked with '-u --accelerated' options
ok 164 - Results not pre-computed
not ok 165 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 622.
#     Structures begin differing at:
#          $got->[0] = 'baker'
#     $expected->[0] = 'golfer'
ok 166 - Results not stored
ok 167 - Constructor worked with '-u -a' options
ok 168 - Results not pre-computed
not ok 169 - Results ok

#   Failed test 'Results ok'
#   at t/04_oo_lists_dual_acc_unsorted.t line 632.
#     Structures begin differing at:
#          $got->[0] = 'baker'
#     $expected->[0] = 'golfer'
ok 170 - Results not stored
# Looks like you failed 17 tests of 170.
Dubious, test returned 17 (wstat 4352, 0x1100)
Failed 17/170 subtests 

Test Summary Report
-------------------
t/04_oo_lists_dual_acc_unsorted.t (Wstat: 4352 Tests: 170 Failed: 17)
  Failed tests:  124-125, 127-128, 130-131, 133-134, 137
                141, 145, 149, 153, 157, 161, 165, 169
  Non-zero exit status: 17
Files=1, Tests=170,  0 wallclock secs ( 0.02 usr  0.01 sys +  0.04 cusr  0.01 csys =  0.08 CPU)
Result: FAIL

I pushed the branch I was working in to my github:
https://github.com/jkeenan/list-compare/tree/paulo-20150702

Can you investigate?

Thank you very much.
Jim Keenan

Fix bug in tests for --unsorted: order of keys(%hash) is not predictable
The test for --unsorted assumed that the order of the keys of a hash
was predictable. The order changes with the Perl interpreter version.

Change the test to use keys(%expected) as the list to compare against,
which is the same as the module is using.
@pauloscustodio

This comment has been minimized.

Show comment
Hide comment
@pauloscustodio

pauloscustodio Jul 4, 2015

Contributor

There was a bug in the t/04_oo_lists_dual_acc_unsorted.t test file: it
assumed the output of keys was predictable, and the particular order in
my Perl interpreter was hard-coded in the test.

Fixed to create the expected list by calling keys on a hash and committed
to #7

On Fri, Jul 3, 2015 at 3:48 AM, James E Keenan notifications@github.com
wrote:

I pulled down a patch version of your pull request for local inspection. I
squashed your commits into one, then substituted 4 spaces for your hard
tabs. I then went to 'make test'. I got failures in one file.

$ prove -vb t/04_oo_lists_dual_acc_unsorted.t
t/04_oo_lists_dual_acc_unsorted.t ..
1..170
ok 1 - constructor returned true value
ok 2 - unsorted: got expected union
...
ok 119 - Constructor worked with '-a' options
ok 120 - Results not pre-computed
ok 121 - Results ok
ok 122 - Results not stored
ok 123 - Constructor worked with '--unsorted' options
not ok 124 - Results pre-computed
not ok 125 - Results ok

Failed test 'Results pre-computed'

at t/04_oo_lists_dual_acc_unsorted.t line 510.

Structures begin differing at:

$got->[1] = 'edward'

$expected->[1] = 'delta'

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 511.

Structures begin differing at:

$got->[1] = 'edward'

$expected->[1] = 'delta'

ok 126 - Constructor worked with '--unsorted' options
not ok 127 - Results pre-computed

Failed test 'Results pre-computed'

at t/04_oo_lists_dual_acc_unsorted.t line 520.

Structures begin differing at:

$got->[0] = 'delta'

$expected->[0] = 'golfer'

not ok 128 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 521.

Structures begin differing at:

$got->[0] = 'delta'

$expected->[0] = 'golfer'

ok 129 - Constructor worked with '-u' options
not ok 130 - Results pre-computed

Failed test 'Results pre-computed'

at t/04_oo_lists_dual_acc_unsorted.t line 530.

Structures begin differing at:

$got->[1] = 'baker'

$expected->[1] = 'delta'

not ok 131 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 531.

Structures begin differing at:

$got->[1] = 'baker'

$expected->[1] = 'delta'

ok 132 - Constructor worked with '-u' options
not ok 133 - Results pre-computed

Failed test 'Results pre-computed'

at t/04_oo_lists_dual_acc_unsorted.t line 540.

Structures begin differing at:

$got->[0] = 'baker'

$expected->[0] = 'golfer'

not ok 134 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 541.

Structures begin differing at:

$got->[0] = 'baker'

$expected->[0] = 'golfer'

ok 135 - Constructor worked with '--accelerated --unsorted' options
ok 136 - Results not pre-computed
not ok 137 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 551.

Structures begin differing at:

$got->[0] = 'camera'

$expected->[0] = 'golfer'

ok 138 - Results not stored
ok 139 - Constructor worked with '--accelerated --unsorted' options
ok 140 - Results not pre-computed
not ok 141 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 562.

Structures begin differing at:

$got->[0] = 'delta'

$expected->[0] = 'golfer'

ok 142 - Results not stored
ok 143 - Constructor worked with '--accelerated -u' options
ok 144 - Results not pre-computed
not ok 145 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 572.

Structures begin differing at:

$got->[1] = 'baker'

$expected->[1] = 'delta'

ok 146 - Results not stored
ok 147 - Constructor worked with '--unsorted --accelerated' options
ok 148 - Results not pre-computed
not ok 149 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 582.

Structures begin differing at:

$got->[0] = 'delta'

$expected->[0] = 'golfer'

ok 150 - Results not stored
ok 151 - Constructor worked with '--unsorted -a' options
ok 152 - Results not pre-computed
not ok 153 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 592.

Structures begin differing at:

$got->[1] = 'fargo'

$expected->[1] = 'delta'

ok 154 - Results not stored
ok 155 - Constructor worked with '-a --unsorted' options
ok 156 - Results not pre-computed
not ok 157 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 602.

Structures begin differing at:

$got->[3] = 'fargo'

$expected->[3] = 'baker'

ok 158 - Results not stored
ok 159 - Constructor worked with '-a -u' options
ok 160 - Results not pre-computed
not ok 161 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 612.

Structures begin differing at:

$got->[2] = 'fargo'

$expected->[2] = 'edward'

ok 162 - Results not stored
ok 163 - Constructor worked with '-u --accelerated' options
ok 164 - Results not pre-computed
not ok 165 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 622.

Structures begin differing at:

$got->[0] = 'baker'

$expected->[0] = 'golfer'

ok 166 - Results not stored
ok 167 - Constructor worked with '-u -a' options
ok 168 - Results not pre-computed
not ok 169 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 632.

Structures begin differing at:

$got->[0] = 'baker'

$expected->[0] = 'golfer'

ok 170 - Results not stored

Looks like you failed 17 tests of 170.

Dubious, test returned 17 (wstat 4352, 0x1100)
Failed 17/170 subtests

Test Summary Report

t/04_oo_lists_dual_acc_unsorted.t (Wstat: 4352 Tests: 170 Failed: 17)
Failed tests: 124-125, 127-128, 130-131, 133-134, 137
141, 145, 149, 153, 157, 161, 165, 169
Non-zero exit status: 17
Files=1, Tests=170, 0 wallclock secs ( 0.02 usr 0.01 sys + 0.04 cusr 0.01 csys = 0.08 CPU)
Result: FAIL

I pushed the branch I was working in to my github:
https://github.com/jkeenan/list-compare/tree/paulo-20150702

Can you investigate?

Thank you very much.
Jim Keenan


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

Contributor

pauloscustodio commented Jul 4, 2015

There was a bug in the t/04_oo_lists_dual_acc_unsorted.t test file: it
assumed the output of keys was predictable, and the particular order in
my Perl interpreter was hard-coded in the test.

Fixed to create the expected list by calling keys on a hash and committed
to #7

On Fri, Jul 3, 2015 at 3:48 AM, James E Keenan notifications@github.com
wrote:

I pulled down a patch version of your pull request for local inspection. I
squashed your commits into one, then substituted 4 spaces for your hard
tabs. I then went to 'make test'. I got failures in one file.

$ prove -vb t/04_oo_lists_dual_acc_unsorted.t
t/04_oo_lists_dual_acc_unsorted.t ..
1..170
ok 1 - constructor returned true value
ok 2 - unsorted: got expected union
...
ok 119 - Constructor worked with '-a' options
ok 120 - Results not pre-computed
ok 121 - Results ok
ok 122 - Results not stored
ok 123 - Constructor worked with '--unsorted' options
not ok 124 - Results pre-computed
not ok 125 - Results ok

Failed test 'Results pre-computed'

at t/04_oo_lists_dual_acc_unsorted.t line 510.

Structures begin differing at:

$got->[1] = 'edward'

$expected->[1] = 'delta'

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 511.

Structures begin differing at:

$got->[1] = 'edward'

$expected->[1] = 'delta'

ok 126 - Constructor worked with '--unsorted' options
not ok 127 - Results pre-computed

Failed test 'Results pre-computed'

at t/04_oo_lists_dual_acc_unsorted.t line 520.

Structures begin differing at:

$got->[0] = 'delta'

$expected->[0] = 'golfer'

not ok 128 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 521.

Structures begin differing at:

$got->[0] = 'delta'

$expected->[0] = 'golfer'

ok 129 - Constructor worked with '-u' options
not ok 130 - Results pre-computed

Failed test 'Results pre-computed'

at t/04_oo_lists_dual_acc_unsorted.t line 530.

Structures begin differing at:

$got->[1] = 'baker'

$expected->[1] = 'delta'

not ok 131 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 531.

Structures begin differing at:

$got->[1] = 'baker'

$expected->[1] = 'delta'

ok 132 - Constructor worked with '-u' options
not ok 133 - Results pre-computed

Failed test 'Results pre-computed'

at t/04_oo_lists_dual_acc_unsorted.t line 540.

Structures begin differing at:

$got->[0] = 'baker'

$expected->[0] = 'golfer'

not ok 134 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 541.

Structures begin differing at:

$got->[0] = 'baker'

$expected->[0] = 'golfer'

ok 135 - Constructor worked with '--accelerated --unsorted' options
ok 136 - Results not pre-computed
not ok 137 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 551.

Structures begin differing at:

$got->[0] = 'camera'

$expected->[0] = 'golfer'

ok 138 - Results not stored
ok 139 - Constructor worked with '--accelerated --unsorted' options
ok 140 - Results not pre-computed
not ok 141 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 562.

Structures begin differing at:

$got->[0] = 'delta'

$expected->[0] = 'golfer'

ok 142 - Results not stored
ok 143 - Constructor worked with '--accelerated -u' options
ok 144 - Results not pre-computed
not ok 145 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 572.

Structures begin differing at:

$got->[1] = 'baker'

$expected->[1] = 'delta'

ok 146 - Results not stored
ok 147 - Constructor worked with '--unsorted --accelerated' options
ok 148 - Results not pre-computed
not ok 149 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 582.

Structures begin differing at:

$got->[0] = 'delta'

$expected->[0] = 'golfer'

ok 150 - Results not stored
ok 151 - Constructor worked with '--unsorted -a' options
ok 152 - Results not pre-computed
not ok 153 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 592.

Structures begin differing at:

$got->[1] = 'fargo'

$expected->[1] = 'delta'

ok 154 - Results not stored
ok 155 - Constructor worked with '-a --unsorted' options
ok 156 - Results not pre-computed
not ok 157 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 602.

Structures begin differing at:

$got->[3] = 'fargo'

$expected->[3] = 'baker'

ok 158 - Results not stored
ok 159 - Constructor worked with '-a -u' options
ok 160 - Results not pre-computed
not ok 161 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 612.

Structures begin differing at:

$got->[2] = 'fargo'

$expected->[2] = 'edward'

ok 162 - Results not stored
ok 163 - Constructor worked with '-u --accelerated' options
ok 164 - Results not pre-computed
not ok 165 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 622.

Structures begin differing at:

$got->[0] = 'baker'

$expected->[0] = 'golfer'

ok 166 - Results not stored
ok 167 - Constructor worked with '-u -a' options
ok 168 - Results not pre-computed
not ok 169 - Results ok

Failed test 'Results ok'

at t/04_oo_lists_dual_acc_unsorted.t line 632.

Structures begin differing at:

$got->[0] = 'baker'

$expected->[0] = 'golfer'

ok 170 - Results not stored

Looks like you failed 17 tests of 170.

Dubious, test returned 17 (wstat 4352, 0x1100)
Failed 17/170 subtests

Test Summary Report

t/04_oo_lists_dual_acc_unsorted.t (Wstat: 4352 Tests: 170 Failed: 17)
Failed tests: 124-125, 127-128, 130-131, 133-134, 137
141, 145, 149, 153, 157, 161, 165, 169
Non-zero exit status: 17
Files=1, Tests=170, 0 wallclock secs ( 0.02 usr 0.01 sys + 0.04 cusr 0.01 csys = 0.08 CPU)
Result: FAIL

I pushed the branch I was working in to my github:
https://github.com/jkeenan/list-compare/tree/paulo-20150702

Can you investigate?

Thank you very much.
Jim Keenan


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

@jkeenan

This comment has been minimized.

Show comment
Hide comment
@jkeenan

jkeenan Jul 4, 2015

Owner

I followed the same procedure as before, i.e., I got the patch version of your changes and applied them to a fresh local branch. When I called 'make test', I continued to get errors in t/04_oo_lists_dual_acc_unsorted.t.

That led me to study your revisions to that test file more closely than I did the first time around (when I was mainly focused on the nested foreach loop pattern). I don't think it is legitimate to probe the internals of the object in the way you are doing. I think you were trying to test more than could legitimately be tested and more than what the documentation claims for the '--unsorted' and '--accelerated' options. I added some inline POD to that test file with my thoughts on that question.

Net result: While I accepted your _parse_options() internal subroutine as a way to provide '--unsorted' and '--accelerated' in either order, I did not accept the tests you wrote in that file. Instead, I substituted some tests which simply test, by implication, the results of _parse_options(). See master branch.

Thank you very much.
Jim Keenan

Owner

jkeenan commented Jul 4, 2015

I followed the same procedure as before, i.e., I got the patch version of your changes and applied them to a fresh local branch. When I called 'make test', I continued to get errors in t/04_oo_lists_dual_acc_unsorted.t.

That led me to study your revisions to that test file more closely than I did the first time around (when I was mainly focused on the nested foreach loop pattern). I don't think it is legitimate to probe the internals of the object in the way you are doing. I think you were trying to test more than could legitimately be tested and more than what the documentation claims for the '--unsorted' and '--accelerated' options. I added some inline POD to that test file with my thoughts on that question.

Net result: While I accepted your _parse_options() internal subroutine as a way to provide '--unsorted' and '--accelerated' in either order, I did not accept the tests you wrote in that file. Instead, I substituted some tests which simply test, by implication, the results of _parse_options(). See master branch.

Thank you very much.
Jim Keenan

@jkeenan jkeenan closed this Jul 4, 2015

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