--- El mié 8-oct-08, Christoph Otto <[EMAIL PROTECTED]> escribió:
De:: Christoph Otto <[EMAIL PROTECTED]>
Asunto: Re: [perl #44457] [TODO] make sure files match test files for DYNPMCs
and DYNOPs etc
A: "Igor" <[EMAIL PROTECTED]>
Cc: [EMAIL PROTECTED]
Fecha: miércoles, 8 octubre, 2008, 11:17 pm
[EMAIL PROTECTED] via RT wrote:
>
> --- El mar 30-sep-08, Christoph Otto <[EMAIL PROTECTED]>
escribió:
> De:: Christoph Otto <[EMAIL PROTECTED]>
> Asunto: Re: [perl #44457] [TODO] make sure files match test files for
DYNPMCs and DYNOPs etc
> A: "Igor ;" <[EMAIL PROTECTED]>
> Cc: [EMAIL PROTECTED]
> Fecha: martes, 30 septiembre, 2008, 5:49 am
>
> Igor wrote:
>> <snip>
>> Hi Christoph,
>> I send you the patch attached.
>> Sincerely,
>> Igor
>
> Hi Igor,
>
> Thanks again for taking the time to contribute.
>
> Here are some pointers:
> First, you're trying too hard. I'm sorry to tell you that after
> you've spent
> so much effort, but this change should be fairly minimal. I'm sure
> you've
> noticed that there are two tests in t/distro/test_file_coverage.t that
almost
> do what you need done. Try copy/pasting code from those tests in the PMC
> block and modifying it to check for the files you're interested in.
> Don't
> worry about the PMC: label. You can give it a more appropriate name once
the
> rest of the test works.
>
> A good rule is to try to use as little code as possible to get the job
done..
> This doesn't mean that you should use weird one-liners that nobody
can
> understand, but that you should strive to write simple, easily understood
> code. Line 88 in the file is a fairly good example of what *not* to do,
> although it'd be ok with a brief explanatory comment.
>
> Also, don't worry too much about generalizing beyond what's needed
to
> get the
> current job done. If the code starts to look ugly, it can be refactored
later.
>
> I hope that sets you in the right direction. Feel free to email me if you
> have any questions.
>
> Christoph
>
> Hi Christoph,
>
> I forgot attach the file.
>
> Sincerely,
>
> Igor
>Hi Igor,
>Patch naming depends on personal preferences, but patches are easier to review
>when their filenames end in either .patch or .diff. Naming different versions
>of a patch with successive numbers (i.e. too_many_frobs_1.patch,
>too_many_frobs_2.patch, etc) makes it easier to see which patch is the most
>recent when using RT, but this isn't critical.
>Also, you only need to reply to [EMAIL PROTECTED] I'm on
>the
>list and will see your message.
>This is a much better patch. It was a good idea to refactor the find commands
>into a separate sub. The code is cleaner and a little bit more obvious, which
>is a good thing.
>My only concern is that it looks like you deleted the tests that matches
>non-dynamic PMCs to their respective tests. Those tests did fail but they
>should be kept in test_file_coverage.t TODO'd, so they can eventually be
>addressed. I may end up doing this myself, so be sure to check if the tests
>actually need to be TODO'd before sending the updated patch.
>Apart from that change, the patch looks good. If you can make those changes
>and resubmit, I'll be glad to apply it.
>Thanks,
>Christoph
Hi Christoph,
I send you the patch atached.
The script with TODO block works equal without it(with TODO
gives more information and adds the "not yet implemented" leyend).
Sincerely,
Igor
¡Todo sobre Amor y Sexo!
La guía completa para tu vida en Mujer de Hoy.
http://mx.mujer.yahoo.com/Index: test_file_coverage.t
===================================================================
--- test_file_coverage.t (revision 31535)
+++ test_file_coverage.t (working copy)
@@ -29,44 +29,58 @@
## make sure PMC files match test files
PMC: {
- my $pmc_dir = 'src/pmc';
- my $pmc_suffix = '.pmc';
- my $test_dir = 't/pmc';
- my $test_suffix = '.t';
+ # Set variables for pmc
+ my $pmc_dir = 'src/pmc';
+ my $pmc_suffix = '.pmc';
+ my $test_pmc_dir = 't/pmc';
+ my $test_pmc_suffix = '.t';
+ my ( $pmc_miss, $test_pmc_miss ) = find_files($pmc_dir,$pmc_suffix,$test_pmc_dir,$test_pmc_suffix);
- my ( @pmc_files, @test_files );
+ # Set variables for dynpmc
+ my $dynpmc_dir = 'src/dynpmc';
+ my $dynpmc_suffix = '.pmc';
+ my $test_dynpmc_dir = 't/dynpmc';
+ my $test_dynpmc_suffix = '.t';
+ my ( $dynpmc_miss, $test_dynpmc_miss ) = find_files($dynpmc_dir,$dynpmc_suffix,$test_dynpmc_dir,$test_dynpmc_suffix);
- # find pmc files
- find {
- no_chdir => 1,
- wanted => sub { files_of_type( [EMAIL PROTECTED], $pmc_suffix ) },
- } => catdir( $PConfig{build_dir}, $pmc_dir );
+ # Set variables for dynoplibs
+ my $dynoplibs_dir = 'src/dynoplibs';
+ my $dynoplibs_suffix = '.ops';
+ my $test_dynoplibs_dir = 't/dynoplibs';
+ my $test_dynoplibs_suffix = '.t';
+ my ( $dynoplibs_miss, $test_dynoplibs_miss ) = find_files($dynoplibs_dir,$dynoplibs_suffix,$test_dynoplibs_dir,$test_dynoplibs_suffix);
- # find test files
- find {
- no_chdir => 1,
- wanted => sub { files_of_type( [EMAIL PROTECTED], $test_suffix ) },
- } => catdir( $PConfig{build_dir}, $test_dir );
+ local $" = "\n\t";
- my ( $pmc_miss, $test_miss ) = list_diff( [EMAIL PROTECTED], [EMAIL PROTECTED] );
+#TODO: {
+# local $TODO = "not yet implemented";
- local $" = "\n\t";
+ # Tests in src/pmc
+ ok( [EMAIL PROTECTED], "there are test files for all PMC files in $pmc_dir" )
+ or diag "files in $test_pmc_dir but not in PMC dir:[EMAIL PROTECTED]";
+ ok( [EMAIL PROTECTED], "there are PMC files for all test files in $test_pmc_dir" )
+ or diag "files in $pmc_dir but not in test dir:[EMAIL PROTECTED]";
-TODO: {
- local $TODO = "not yet implemented";
- ok( [EMAIL PROTECTED], "there are test files for all PMC files in $pmc_dir" )
- or diag "files in $test_dir but not in PMC dir:[EMAIL PROTECTED]";
- }
- ok( [EMAIL PROTECTED], "there are PMC files for all test files in $test_dir" )
- or diag "files in $pmc_dir but not in test dir:[EMAIL PROTECTED]";
+ # Tests in src/dynpmc
+ ok( [EMAIL PROTECTED], "there are test files for all PMC files in $dynpmc_dir" )
+ or diag "files in $test_dynpmc_dir but not in PMC dir:[EMAIL PROTECTED]";
+ ok( [EMAIL PROTECTED], "there are PMC files for all test files in $test_dynpmc_dir" )
+ or diag "files in $dynpmc_dir but not in test dir:[EMAIL PROTECTED]";
+ # Tests in src/dynoplibs
+ ok( [EMAIL PROTECTED], "there are test files for all OPS files in $dynoplibs_dir" )
+ or diag "files in $test_dynoplibs_dir but not in OPS dir:[EMAIL PROTECTED]";
+ ok( [EMAIL PROTECTED], "there are OPS files for all test files in $test_dynoplibs_dir" )
+ or diag "files in $dynoplibs_dir but not in test dir:[EMAIL PROTECTED]";
+# }
+
} # PMC
# RT#44457: DYNPMC, DYNOPS, etc.
# remember to change the number of tests :-)
-BEGIN { plan tests => 2; }
+BEGIN { plan tests => 6; }
sub files_of_type {
my ( $listref, $ext ) = @_;
@@ -89,6 +103,30 @@
);
}
+sub find_files {
+
+ my ($type_dir,$type_suffix,$test_dir,$test_suffix) = @_;
+
+ my ( @type_files, @test_files );
+
+ # find suffix type files
+ find {
+ no_chdir => 1,
+ wanted => sub { files_of_type( [EMAIL PROTECTED], $type_suffix ) },
+ } => catdir( $PConfig{build_dir}, $type_dir );
+
+ # find test files
+ find {
+ no_chdir => 1,
+ wanted => sub { files_of_type( [EMAIL PROTECTED], $test_suffix ) },
+ } => catdir( $PConfig{build_dir}, $test_dir );
+
+ my ( $type, $test ) = list_diff( [EMAIL PROTECTED], [EMAIL PROTECTED] );
+
+ return ( $type, $test );
+
+}
+
# Local Variables:
# mode: cperl
# cperl-indent-level: 4
