Ticket #1272 (closed RFC: done)

Opened 5 years ago

Last modified 4 years ago

Problems with codetest on files in ext/ directory (should ext/ be included in codetest?)

Reported by: mikehh Owned by:
Priority: normal Milestone:
Component: testing Version: 1.7.0
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description

I have recently been getting codetest failures with t/codingstd/pod_syntax.t in ext/nqp-rx

I did fix the files in question (redundant =cut at the end) but found that there were a whole bunch of trailing spaces that my editor removed on saving the file that were not picked up by the trailing spaces test.

Subsequent commits to that directory did not include the fixes I made and I was required to redo the fixes.

The situation is that we should either run all codingstd tests on the files in the ext directory or none.

We should bear in mind that the files in the ext directory are external to parrot.

pmichaud++ stated that the files are generated and that it would be difficult to fix the generator to prevent the generation of trailing spaces.

also:

<pmichaud> mikehh: imo, we shouldn't be running codetest on things in ext/
<pmichaud> mikehh: if you're editing the files in ext/nqp-rx, it won't do much good -- they get overwritten by newer versions

The problem is that the fixes are not 'permanent' and the next time the code is committed to parrot, if the fixes have not been included upstream the failure re-occurs.

Change History

Changed 5 years ago by jkeenan

Each file in t/codingstd/ contains its own specification of the files it will test. In most cases, these are determined by calling some combination of methods from Parrot::Distribution. Example: t/codingstd/cuddled_else.t:

my $DIST = Parrot::Distribution->new;
my @files = @ARGV ? <@ARGV> : (
    $DIST->get_c_language_files(),
    $DIST->get_perl_language_files(),
);

Other test files hand-roll the list of source code files they test. Example: t/codingstd/linelength.t:

my $build_dir = $PConfig{build_dir};
my $manifest = maniread( File::Spec->catfile( $build_dir, 'MANIFEST' ) );

# skip files listed in the __DATA__ section
my %skip_files;
while (<DATA>) {
    next if m{^#};
    next if m{^\s*$};
    chomp; 
    $skip_files{$_}++;
}

The POD coding standard test rely on Parrot::Test::Pod, which parses MANIFEST and MANIFEST.generated.

So it doesn't seem that there is an easy way of specifying in only one location that an entire subdirectory should be excluded from the scope of make codetest.

Perhaps in the short term we should focus on the question: Which codingstd tests are most likely to fail whenever files in ext/nqp-rx are updated? We can then get quick fixes on the most annoying cases.

Thank you very much.

kid51

Changed 5 years ago by coke

Short term solutions aside, +1 on the long term goal of excluding ext/ from our codingstd tests.

Changed 5 years ago by allison

+1 on long-term excluding ext/ from the codingstd tests.

Changed 4 years ago by mikehh

  • status changed from new to closed
  • resolution set to done

This ticket seems to have been resolved.

Closing

Note: See TracTickets for help on using tickets.