On Sun, Mar 15, 2020 at 07:35:46AM -0400, Michael S. Tsirkin wrote:
> If the process documented in tests/qtest/bios-tables-test.c
> is followed, then same patch never touches both expected
> files and code. Teach checkpatch to enforce this rule.
> 
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> ---
> 
> Peter, Igor what do you think?

Minor comments below:

Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>

> 
>  scripts/checkpatch.pl | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index b27e4ff5e9..96583e3fff 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -35,6 +35,8 @@ my $summary_file = 0;
>  my $root;
>  my %debug;
>  my $help = 0;
> +my $testexpected;
> +my $nontestexpected;

If you respin, please add acpi to these variable names since they are
specific to acpi.

>  
>  sub help {
>       my ($exitcode) = @_;
> @@ -1256,6 +1258,26 @@ sub WARN {
>       }
>  }
>  
> +# According to tests/qtest/bios-tables-test.c: do not
> +# change expected file in the same commit with adding test
> +sub checkfilename {
> +     my ($name) = @_;

There is a tab instead of spaces here that could be fixed if you decide
to respin.

> +        if ($name =~ m#^tests/data/acpi/# and
> +            # make exception for a shell script that rebuilds the files
> +            not $name =~ m#^\.sh$# or
> +            $name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
> +            $testexpected = $name;
> +        } else {
> +            $nontestexpected = $name;
> +        }
> +        if (defined $testexpected and defined $nontestexpected) {
> +            ERROR("Do not add expected files together with tests, " .
> +                  "follow instructions in " .
> +                  "tests/qtest/bios-tables-test.c: both " .
> +                  $testexpected . " and " . $nontestexpected . " found\n");
> +        }
> +}
> +
>  sub process {
>       my $filename = shift;
>  
> @@ -1431,9 +1453,11 @@ sub process {
>               if ($line =~ /^diff --git.*?(\S+)$/) {
>                       $realfile = $1;
>                       $realfile =~ s@^([^/]*)/@@ if (!$file);
> +                        checkfilename($realfile);
>               } elsif ($line =~ /^\+\+\+\s+(\S+)/) {
>                       $realfile = $1;
>                       $realfile =~ s@^([^/]*)/@@ if (!$file);
> +                        checkfilename($realfile);

The surrounding lines in this hunk use tab indentation, not spaces.

Attachment: signature.asc
Description: PGP signature

Reply via email to