search

redhotpenguin / perl-Archive-Zip

Archive::Zip as seen at https://metacpan.org/pod/Archive::Zip
Other
15 stars 44 forks source link

Several problems in "make test" #54

Closed farblos closed 5 years ago

farblos commented 5 years ago

There are a number of problems in the current test suite (1.66 + some PRs):

While look into these ASAP.

farblos commented 5 years ago

Can reproduce failing test t/26_bug_26.t locally but don't have the time to look into more details now. @pmqs, any opinion on that one?

No idea yet what to do about last problem ("zip doesn't seem to work") but to add diagnostic output.

pmqs commented 5 years ago
  • there are tests t/26_symlink.t and t/26_bug_26.t. One of them should be renumbered for clarity.
  • test t/26_bug_26.t is not mentioned in MANIFEST and not packaged, hence

Created PR #55 to address the issues above.

pmqs commented 5 years ago

Can reproduce failing test t/26_bug_26.t locally but don't have the time to look into more details now. @pmqs, any opinion on that one?

will try to get a look at.

No idea yet what to do about last problem ("zip doesn't seem to work") but to add diagnostic output.

I'm not seeing that error.

farblos commented 5 years ago

I'm not seeing that error.

Agreed. Must have been an intermittent issue, to be seen only in the one log file I've quoted above.

farblos commented 5 years ago 
* test `t/26_bug_26.t` fails for yet unknown reasons

@pmqs: Seems that the zip64 PR #45 has thrown out the changes in Member.pm of your bzip compression commit 9122328b9e187de23c1e4d1be5d6259cefa9248e. (I remember having eliminated parameter $mode to function head because the only value ever passed to that method before commit 9122328b9e187de23c1e4d1be5d6259cefa9248e was 1.)

I guess that this could be the root cause, but don't have time to validate it now.

Um, probably you could add a one-liner to test 26_bzip.t in your PR #54 to provide more details what the test should actually do?

pmqs commented 5 years ago

@pmqs: Seems that the zip64 PR #45 has thrown out the changes in Member.pm of your bzip compression commit 9122328. (I remember having eliminated parameter $mode to function head because the only value ever passed to that method before commit 9122328 was 1.)

I guess that this could be the root cause, but don't have time to validate it now.

yes, I that is exactly why the test is failing.

Um, probably you could add a one-liner to test 26_bzip.t in your PR #54 to provide more details what the test should actually do?

Done

redhotpenguin commented 5 years ago

Thanks. Will push 1.67 this weekend.

On Fri, Sep 20, 2019, 3:26 PM Paul Marquess notifications@github.com wrote:

@pmqs https://github.com/pmqs: Seems that the zip64 PR #45 https://github.com/redhotpenguin/perl-Archive-Zip/pull/45 has thrown out the changes in Member.pm of your bzip compression commit 9122328 https://github.com/redhotpenguin/perl-Archive-Zip/commit/9122328b9e187de23c1e4d1be5d6259cefa9248e. (I remember having eliminated parameter $mode to function head because the only value ever passed to that method before commit 9122328 https://github.com/redhotpenguin/perl-Archive-Zip/commit/9122328b9e187de23c1e4d1be5d6259cefa9248e was 1.)

I guess that this could be the root cause, but don't have time to validate it now.

yes, I that is exactly why the test is failing.

Um, probably you could add a one-liner to test 26_bzip.t in your PR #54 https://github.com/redhotpenguin/perl-Archive-Zip/issues/54 to provide more details what the test should actually do?

Done

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/redhotpenguin/perl-Archive-Zip/issues/54?email_source=notifications&email_token=AAAXRZN4RJKMSCVKFK6WO7LQKVEYDA5CNFSM4IYWJJ52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7IBIXQ#issuecomment-533730398, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAXRZJEZNDZOT2NNLD2FJTQKVEYDANCNFSM4IYWJJ5Q .

redhotpenguin commented 5 years ago

Sorry - maybe I read too fast here. Looks like there's still some investigation going. LMK when a release is needed.

farblos commented 5 years ago

To summarize, we have the following issues and PRs that I consider worth including in the next release:

pmqs commented 5 years ago

To summarize, we have the following issues and PRs that I consider worth including in the next release:

  • this issue, fixed by pmqs' PR #55 and #57
  • issue #51, fixed by PR #53
  • issue #56

Agree

farblos commented 5 years ago

All problems fixed in version 1.67.