Bug #6070 for CPANPLUS: Mail Message text changes

Mon Apr 19 12:48:43 2004 BARBIE [...] cpan.org - Ticket created

Subject:

Mail Message text changes

Andy Lester (PETDANCE) recently reported, on the CPAN Testers mailing list, that the mail sent to authors following a FAIL or UNKNOWN report was a bit misleading, particularly for new authors. As such I have rewritten the contents of the mails produced to be a bit less intimidating and more likely to produce a positive response. Patch attached. CPANPLUS-0.048 Perl 5.6.1 Windows 2000 Pro

--- C:\wip\diffs\CPANPLUS-0.048\CPANPLUS\Internals/Report.pm Sun Oct 5 18:15:18 2003 +++ C:\wip\diffs\CPANPLUS-0.048\CPANPLUS\Internals/Report-barbie.pm Thu Apr 1 13:36:57 2004 @@ -125,14 +125,21 @@ and ($stage !~ /\btest\b/); $text = << "."; -This is an error report generated automatically by CPANPLUS, -version $VERSION. +Dear $author, + +This is a computer-generated error report created automatically by +CPANPLUS, version $VERSION. Testers personal comments may appear at +the end of this report. +Thank you for uploading your works to CPAN. However, it appears there +were some problems testing your distribution. + +TEST RESULTS: + Below is the error stack during '$stage': $buffer -Additional comments: . my %missing; @@ -148,9 +155,9 @@ $text .= << "."; -Hello, $author! Thanks for uploading your works to CPAN. +MISSING PREREQUISITES: -I noticed that the test suite seem to fail without these modules: +It was noticed that the test suite seem to fail without these modules: $modules @@ -174,18 +181,24 @@ ******************************** NOTE ******************************** The comments above are created mechanically, possibly without manual -checking by the sender. Also, because many people perform automatic -tests on CPAN, chances are that you will receive identical messages -about the same problem. +checking by the sender. As there are many people performing automatic +tests on each upload to CPAN, it is likely that you will receive +identical messages about the same problem. If you believe that the message is mistaken, please reply to the first -one with correction and/or additional information, and do not take +one with correction and/or additional informations, and do not take it personally. We appreciate your patience. :) ********************************************************************** . } + $text .= << "."; + +Additional comments: + +. + push @inform, "$module->{author}\@cpan.org" if (!$already_sent or $nb_send < $max_send); } @@ -198,21 +211,27 @@ my $stage = 'make test'; $text = << "."; -This is an error report generated automatically by CPANPLUS, -version $VERSION. +Dear $author, + +This is a computer-generated error report created automatically by +CPANPLUS, version $VERSION. Testers personal comments may appear at +the end of this report. +Thank you for uploading your works to CPAN. However, it appears there +were some problems testing your distribution. + +TEST RESULTS: + Below is the error stack during '$stage': $buffer -Additional comments: +RECOMMENDATIONS: -Hello, $author! Thanks for uploading your works to CPAN. +It would be very helpful if you could include even a simple test +script in the next release, so people can verify which platforms +can successfully install them, as well as avoid regression bugs? -Would it be too much to ask for a simple test script in the next -release, so people can verify which platforms can successfully -install them, as well as avoid regression bugs? - A simple 't/use.t' that says: #!/usr/bin/env perl -w @@ -224,22 +243,26 @@ exit; __END__ -would be appreciated. If you are interested in making a more robust -test suite, please see the Test::Simple, Test::More and Test::Tutorial -manpages at <http://search.cpan.org/dist/Test-Simple/>. +would be very much appreciated. If you would like to find out more +about creating a more robust test suite, please see the documentation +that accompanies Test::Simple, Test::More and Test::Tutorial at +<http://search.cpan.org/dist/Test-Simple/>. Thanks! :-) ******************************** NOTE ******************************** The comments above are created mechanically, possibly without manual -checking by the sender. Also, because many people performs automatic -tests on CPAN, chances are that you will receive identical messages -about the same problem. +checking by the sender. As there are many people performing automatic +tests on each upload to CPAN, it is likely that you will receive +identical messages about the same problem. If you believe that the message is mistaken, please reply to the first one with correction and/or additional informations, and do not take it personally. We appreciate your patience. :) ********************************************************************** + + +Additional comments: .

Mon Aug 16 14:19:11 2004 kane [...] cpan.org - Correspondence added

Thanks for this mail and first off, i'd like to say i'm happy to take the patch for a re-wording. However, this is probably best done against the 0.050 texts, which i hope you'll allow me to mail you a bit down the line.

Thu Nov 11 09:05:19 2004 kane [...] cpan.org - Correspondence added

[KANE - Mon Aug 16 14:19:11 2004]: Show quoted text

> Thanks for this mail and first off, i'd like to say i'm happy to take > the patch for a re-wording. > However, this is probably best done against the 0.050 texts, which i > hope you'll allow me to > mail you a bit down the line.

You can find a development tarball of cpanplus here: http://p4.elixus.org/snap/cpanplus-devel.tar.gz And the file to patch against would be: lib/CPANPLUS/Internals/Constants/Report.pm The texts will not change before the release, save your patches to them.

Sun Dec 26 10:32:57 2004 kane [...] cpan.org - Status changed from 'new' to 'stalled'

Fri Jan 07 12:37:27 2005 The RT System itself - Status changed from 'stalled' to 'open'

Fri Jan 07 12:37:27 2005 BARBIE [...] cpan.org - Correspondence added

From:

BARBIE

The attached patch file is to be used against CPANPLUS-0.050_04. The two Report.pm files have been changed to alter the text and ordering. Hope they are suitable.

--- C:\wip\diffs\CPANPLUS-0.050\CPANPLUS-0.050_04/lib/CPANPLUS/Internals/Constants/Report.pm Sun Dec 19 17:39:48 2004 +++ C:\wip\diffs\CPANPLUS-0.050\CPANPLUS-0.050_04-barbie/lib/CPANPLUS/Internals/Constants/Report.pm Fri Jan 7 17:29:41 2005 @@ -107,7 +107,7 @@ => sub { my $buffer = shift or return; return $buffer =~ - /^'?No tests defined for .* extension.\s*$/m + /^'?No tests defined(?: for .* extension).\s*$/m and ( $buffer !~ /\*\.t/m and $buffer !~ /test\.pl/m); @@ -134,24 +134,15 @@ use constant REPORT_MESSAGE_HEADER => sub { - my ($author,$email) = @_; + my $author = shift; return << "."; -******************************** NOTE ******************************** -The comments below are created mechanically, possibly without manual -checking by the sender. As there are many people performing automatic -tests on each upload to CPAN, it is likely that you will receive -identical messages about the same problem. +Dear $author, + +This is a computer-generated error report created automatically by +CPANPLUS, version $CPANPLUS::Internals::VERSION. Testers personal +comments may appear at the end of this report. -If you believe that the message is mistaken, please reply to the first -one with correction and/or additional information, and do not take -it personally. (We appreciate your patience. :) -********************************************************************** - -Hello, $author! This is a computer-generated error report created -automatically by CPANPLUS, version $CPANPLUS::Internals::VERSION. -Comments from the tester may appear at the end of this report, as well. - . }; @@ -160,8 +151,8 @@ my($version, $stage, $buffer) = @_; return << "."; -Thanks for uploading your works to CPAN. However, it appears that -CPANPLUS encountered problems while testing your distribution. +Thank you for uploading your work to CPAN. However, it appears that +were some problems testing your distribution. TEST RESULTS: @@ -174,7 +165,8 @@ use constant REPORT_MISSING_PREREQS => sub { - my @missing = @_; + my ($author,$email,@missing) = @_; + $author = $author ? "$author ($email)" : 'Your Name Here'; my $modules = join "\n", @missing; my $prereqs = join "\n", map {"\t'$_'\t=> '0',". " # or a minimum working version"} @missing; @@ -191,7 +183,7 @@ Makefile.PL should solve this problem. For example: WriteMakefile( - AUTHOR => 'Your Name Here', + AUTHOR => '$author', ... # other information PREREQ_PM => { $prereqs @@ -203,7 +195,7 @@ at <http://search.cpan.org/dist/ExtUtils-AutoInstall/> may be worth a look. -Thanks! +Thanks! :-) . }; @@ -211,10 +203,13 @@ use constant REPORT_MISSING_TESTS => sub { return << "."; -Would it be too much to ask for a simple test script in the next -release, so people can verify which platforms can successfully -install them, as well as avoid regression bugs? +RECOMMENDATIONS: + +It would be very helpful if you could include even a simple test +script in the next release, so people can verify which platforms +can successfully install them, as well as avoid regression bugs? + A simple 't/use.t' that says: #!/usr/bin/env perl -w @@ -228,9 +223,30 @@ would be appreciated. If you are interested in making a more robust test suite, please see the Test::Simple, Test::More and Test::Tutorial -manpages at <http://search.cpan.org/dist/Test-Simple/>. +documentation at <http://search.cpan.org/dist/Test-Simple/>. -Thanks! +Thanks! :-) + +. + }; + + +use constant REPORT_MESSAGE_FOOTER + => sub { + return << "."; + +******************************** NOTE ******************************** +The comments above are created mechanically, possibly without manual +checking by the sender. As there are many people performing automatic +tests on each upload to CPAN, it is likely that you will receive +identical messages about the same problem. + +If you believe that the message is mistaken, please reply to the first +one with correction and/or additional informations, and do not take +it personally. We appreciate your patience. :) +********************************************************************** + +Additional comments: . }; --- C:\wip\diffs\CPANPLUS-0.050\CPANPLUS-0.050_04/lib/CPANPLUS/Internals/Report.pm Sun Dec 26 16:47:16 2004 +++ C:\wip\diffs\CPANPLUS-0.050\CPANPLUS-0.050_04-barbie/lib/CPANPLUS/Internals/Report.pm Fri Jan 7 17:26:38 2005 @@ -330,13 +330,16 @@ ### was it missing prereqs? ### if( my @missing = MISSING_PREREQS_LIST->($buffer) ) { - $message .= REPORT_MISSING_PREREQS->(@missing); + $message .= REPORT_MISSING_PREREQS->($author,$email,@missing); } ### was it missing test files? ### if( NO_TESTS_DEFINED->($buffer) ) { $message .= REPORT_MISSING_TESTS->(); } + + ### the footer + $message = REPORT_MESSAGE_FOOTER->(); } ### if it failed, and that already got reported, we're not cc'ing the

Mon Jan 10 11:15:50 2005 kane [...] cpan.org - Correspondence added

[BARBIE - Fri Jan 7 12:37:27 2005]: Show quoted text

> The attached patch file is to be used against CPANPLUS-0.050_04. The two > Report.pm files have been changed to alter the text and ordering. Hope > they are suitable.

I'm afraid the test suite breaks quite heavily with these patches... below is my output, but it seems that a patch to the .t file will also be required in order to apply these patches: [kane@myriad ~...cpanplus/devel]$ perlc -Ilib t/40_CPANPLUS-Internals-Report.t 1 1..67 [MSG] Checking if source files are up to date [MSG] Retrieving /Users/kane/sources/p4/cpanplus/devel/t/dummy-cpanplus/ sourcefiles.2.09.stored ok 1 - Got proper mail account ok 2 - Test is relevant ok 3 - Test is irrelevant ok 4 - Perl version too low ok 5 - Perl version adequate ok 6 - Tests defined ok 7 - No tests defined ok 8 - Proper test fail stage found ok 9 - Proper test fail stage found ok 10 - List of missing prereqs found ok 11 - Proper prereq found ok 12 - Test header generated not ok 13 - Proper content found # Failed test (t/40_CPANPLUS-Internals-Report.t at line 119) # ' # Dear foo, # # This is a computer-generated error report created automatically by # CPANPLUS, version 0.050_04. Testers personal # comments may appear at the end of this report. # # ' # doesn't match '(?-xism:NOTE)' ok 14 - Proper content found ok 15 - Proper content found ok 16 - Proper content found ok 17 - Test header generated not ok 18 - Proper content found # Failed test (t/40_CPANPLUS-Internals-Report.t at line 128) # ' # Thank you for uploading your work to CPAN. However, it appears that # were some problems testing your distribution. # # TEST RESULTS: # # Below is the error stack from stage 'test': # # buffer # # ' # doesn't match '(?-xism:CPANPLUS)' ok 19 - Proper content found ok 20 - Proper content found ok 21 - Test output generated ok 22 - Proper content found ok 23 - Proper content found ok 24 - Proper content found ok 25 - Missing test string generated ok 26 - Proper content found ok 27 - Proper content found [MSG] Fetching: 'http://testers.cpan.org/show/Text-Bastardize.yaml' ok 28 - Fetched test report ok 29 - Return value has hashrefs ok 30 - Has a grade ok 31 - Grade as expected ok 32 - Has a dist ok 33 - Dist as expected ok 34 - Has a platform Undefined subroutine &CPANPLUS::Internals::Report::REPORT_MESSAGE_FOOTER called at / Users/kane/sources/p4/cpanplus/devel/t/../lib/CPANPLUS/Internals/Report.pm line 342. # Looks like you planned 67 tests but only ran 34. # Looks like your test died just after 34. [MSG] Writing state information back to disk. This may take a little while.

Mon Jan 10 12:35:49 2005 BARBIE [...] cpan.org - Correspondence added

From:

Barbie

Oops sorry forgot about the test files. I have will hopefully have a more robust patch available by the end of the week.

Thu Jan 20 13:34:36 2005 Guest - Correspondence added

Right I have now gone through an fixed the test script :) I've also added a few additional report changes that weren't in the original, however, the complete list of changes are: Major Changes: 1) Check for failure due uninstall external libraries 2) Check whether missing pre-requisites have been listed in the Makefile.PL/Build.PL 3) Reworded parts of the text that is used in the reports to be more friendly 4) Now have a standard footer, where testers can add their comments 5) Checking for platform specific distributions, it should be noted that MAC distributions are not Mac distributions. Minor change: 6) works with Test::Reporter 1.20 Note that this patched against 0.051

Thu Jan 20 13:36:20 2005 BARBIE [...] cpan.org - Correspondence added

From:

barbie [...] missbarbell.co.uk

And it would REALLY help if I added the patch before I submitted too!

Message body is not shown because it is too large.

Fri Jan 21 09:24:11 2005 kane [...] dwim.org - Correspondence added

From:	"Jos I. Boumans" <kane [...] dwim.org>
Subject:	Re: [Cpanplus-bugs] [cpan #6070] Mail Message text changes
Date:	Fri, 21 Jan 2005 15:20:14 +0100
To:	bug-CPANPLUS [...] rt.cpan.org
RT-Send-Cc:

On Jan 20, 2005, at 7:36 PM, via RT wrote: Show quoted text

> > This message about CPANPLUS was sent to you by BARBIE > <BARBIE@cpan.org> via rt.cpan.org > > Full context and any attached attachments can be found at: > <URL: https://rt.cpan.org/Ticket/Display.html?id=6070 > > > And it would REALLY help if I added the patch before I submitted too!

Thanks for submitting.. i have a few nits about the patch though, that i hope you'll correct, before i can apply it: --- C:\wip\diffs\CPANPLUS\CPANPLUS-0.051/lib/CPANPLUS/Internals/Constants/ Report.pm Thu Jan 13 06:09:56 2005 --- C:\wip\diffs\CPANPLUS\CPANPLUS-0.051/lib/CPANPLUS/Internals/Report.pm Thu Jan 13 06:09:56 2005 +++ C:\wip\diffs\CPANPLUS\CPANPLUS-0.051-barbie/lib/CPANPLUS/Internals/ Report.pm Thu Jan 20 18:17:48 2005 @@ -7,6 +7,7 @@ use CPANPLUS::Internals::Constants::Report; use Data::Dumper; +use File::Slurp; use Params::Check qw[check]; use Locale::Maketext::Simple Class => 'CPANPLUS', Style => 'gettext'; This one's not ok -- We can not use File::Slurp as it is not core perl. Please read in the file using a standard do $fh block or something similar. If you insist on File::Slurp, it must be in the conditional load section, and a patch to Bundle::CPANPLUS::Test::Reporter is also in order. @@ -40,7 +41,7 @@ my $send_list = { %$query_list, - 'Test::Reporter' => 1.21, + 'Test::Reporter' => 1.20, }; sub _have_query_report_modules { There are other bugs with T::R, that nowadays require 1.24 (however, 1.21 - 1.23 are already deleted from cpan, so this number is still 'accurate') @@ -314,6 +315,12 @@ my $message; if( $grade eq GRADE_FAIL or $grade eq GRADE_UNKNOWN) { + ### return if one or more missing external libraries + if( my @missing = MISSING_EXTLIBS_LIST->($buffer) ) { + msg(loc("Not sending test report - external libraries not pre-installed")); + return 1; + } + ### will be 'fetch', 'make', 'test', 'install', etc ### my $stage = TEST_FAIL_STAGE->($buffer); Nice one :) @@ -436,6 +449,43 @@ return; } } + +sub _verify_missing_prereqs { + my ($self,$dist,@missing) = @_; + my %missing = map {$_ => 1} @missing; + my $conf = $self->configure_object; + + ### Read pre-requisites from Makefile.PL or Build.PL (if there is one), + ### of the form: + ### 'PREREQ_PM' => { + ### 'Compress::Zlib' => '1.20', + ### 'Test::More' => 0, + ### }, + ### Build.PL uses 'requires' instead of 'PREREQ_PM'. Note that it is perl code being executed -- this is not always going to be accureate... but perhaps the few cases it is catching is well worth adding it in :) + + my $makefile = File::Spec->catdir( $conf->get_conf('base'), + $self->_perl_version( perl => $^X ), + $conf->_get_build('distdir'), + $dist, + 'Makefile.PL'); + my $buildfile = File::Spec->catdir( $conf->get_conf('base'), + $self->_perl_version( perl => $^X ), + $conf->_get_build('distdir'), + $dist, + 'Build.PL'); This is not necessarily correct, and definately cumbersome. Try using: MAKEFILE_PL->( $self->status->extract ) and BUILD_PL->( $self->status->extract ); --- C:\wip\diffs\CPANPLUS\CPANPLUS-0.051/t/40_CPANPLUS-Internals-Report.t Thu Jan 13 06:10:00 2005 +++ C:\wip\diffs\CPANPLUS\CPANPLUS-0.051-barbie/t/40_CPANPLUS-Internals- Report.t Thu Jan 20 18:29:11 2005 @@ -17,7 +17,7 @@ my $query_tests = 7; my $total_tests = $send_tests + $query_tests; -use Test::More tests => 67; +use Test::More tests => 84; use Module::Load::Conditional qw[can_load]; Does this report the proper amount of tests skipped if test reporting is disabled still? Thanks for the effort, i look forward to applying this patch! -- Jos Boumans 'Real programmers use "cat > a.out"' CPANPLUS http://cpanlus.sf.net

Fri Jan 21 09:39:31 2005 BARBIE [...] cpan.org - Correspondence added

From:

barbie [...] missbarbell.co.uk

[kane@dwim.org - Fri Jan 21 09:24:11 2005]: Show quoted text

> Thanks for submitting.. i have a few nits about the patch though, that > i hope you'll correct, before i can apply it:

Will do. I'll hopefully resubmit before the end of the day.

Fri Jan 21 10:19:41 2005 BARBIE [...] cpan.org - Correspondence added

From:

barbie [...] missbarbell.co.uk

Okay new patch. I've removed the dependancy on File::Slurp and now use the _get_file_contents() function within CPANPLUS::Internals::Utils. I've also added the parameter check bit to my new function _verify_missing_prereqs(). Returned the Test::Reporter version back to 1.21.

Message body is not shown because it is too large.

Sun Jan 23 08:18:18 2005 kane [...] cpan.org - Status changed from 'open' to 'resolved'

Sun Jan 23 08:18:18 2005 kane [...] cpan.org - Correspondence added

[BARBIE - Fri Jan 21 10:19:41 2005]: Show quoted text

> Okay new patch. > > I've removed the dependancy on File::Slurp and now use the > _get_file_contents() function within CPANPLUS::Internals::Utils. I've > also added the parameter check bit to my new function > _verify_missing_prereqs(). > > Returned the Test::Reporter version back to 1.21.

Thanks, applied with several tweaks as 11914