Add a simple perl lint script and fix all the errors it found

This adds a script that checks for common whitespace issues
(per our house style) and can (if run with --write) correct some
of them. It adds a run of the script (without --write) to the
tox config, and includes fixes for all the issues the script
discovered in the existing tests and libs.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

lib/anaconda.pm

@@ -467,5 +467,4 @@ sub report_bug_text {
 
     # Quit anaconda
     type_string "4\n";
-    
 }

lib/fedoradistribution.pm

@@ -75,14 +75,14 @@ sub ensure_installed {
     # We will check if GUI elements are present, that would suggest that we are not in the
     # console but in GUI.
     if (check_screen("apps_menu_button")) {
-	# In that case, we want to return to GUI after the routine finishes.
+        # In that case, we want to return to GUI after the routine finishes.
         $stay_on_console = 0;
-	    # From GUI we need to switch to the console.
+        # From GUI we need to switch to the console.
-	    send_key("ctrl-alt-f3");
+        send_key("ctrl-alt-f3");
-	# Let's wait to allow for screen changes.
+        # Let's wait to allow for screen changes.
-    sleep 5;
+        sleep 5;
-    # And do the login.
+        # And do the login.
-	console_login();
+        console_login();
     }
     # Try to install the packages via dnf. If it is already installed, DNF will not do anything
     # so there is no need to do any complicated magic.
@@ -102,21 +102,21 @@ sub become_root {
     # If ROOT_PASSWORD exists, it means that the root account exists, too.
     # To become root, we will use the real root account and we'll switch to it.
     if (check_var("ROOT_PASSWORD")) {
-	my $password = get_var("ROOT_PASSWORD");
+        my $password = get_var("ROOT_PASSWORD");
-	enter_cmd("su -", max_interval => 15, wait_screen_changes => 3);
+        enter_cmd("su -", max_interval => 15, wait_screen_changes => 3);
-	type_password($password, max_interval => 15);
+        type_password($password, max_interval => 15);
-	send_key("ret");
+        send_key("ret");
     }
     # If no root password is set, it means, that we are only using an administrator
     # who is in the wheel group and therefore we will use the sudo command to obtain
     # the admin rights.
     else {
         my $password = get_var("USER_PASSWORD") || "weakpassword";
-	    enter_cmd("sudo -i", max_interval => 15, wait_screen_changes => 3);
+        enter_cmd("sudo -i", max_interval => 15, wait_screen_changes => 3);
-	    # The SUDO warning might be displayed so let's wait it out a bit.
+        # The SUDO warning might be displayed so let's wait it out a bit.
-	    sleep 2;
+        sleep 2;
-	    type_password($password, max_interval => 15);
+        type_password($password, max_interval => 15);
-	    send_key("ret");
+        send_key("ret");
     }
     sleep 2;
     # Now we should be root. Let's check for root prompt.
@@ -144,9 +144,9 @@ sub script_sudo {
 
     my $str;
     if ($wait > 0) {
-	# Create a uniqe hash from the command and the wait time.
+        # Create a uniqe hash from the command and the wait time.
         $str  = testapi::hashed_string("SS$prog$wait");
-	# Chain the commands to pass the message to the serial console.
+        # Chain the commands to pass the message to the serial console.
         $prog = "$prog; echo $str > /dev/$testapi::serialdev";
     }
     # Run the command with `sudo -k`

lib/utils.pm

@@ -82,14 +82,14 @@ sub boot_to_login_screen {
         # The following is true for non-serial console.
         my $count = 5;
         while (check_screen("login_screen", 3) && $count > 0) {
-           sleep 5;
+            sleep 5;
-           $count -= 1;
+            $count -= 1;
+        }
+        assert_screen "login_screen", $args{timeout};
+        if (match_has_tag "graphical_login") {
+            wait_still_screen 10, 30;
+            assert_screen "login_screen";
         }
-       assert_screen "login_screen", $args{timeout};
-       if (match_has_tag "graphical_login") {
-           wait_still_screen 10, 30;
-           assert_screen "login_screen";
-       }
     }
 }
 
@@ -763,7 +763,7 @@ sub gnome_initial_setup {
     # wait a bit in case of animation
     wait_still_screen 3;
     # one more check for frickin auth_required
-   if (check_screen "auth_required") {
+    if (check_screen "auth_required") {
         record_soft_failure "Unexpected authentication required: https://gitlab.gnome.org/GNOME/gnome-initial-setup/-/issues/106";
         send_key "esc";
     }
@@ -941,7 +941,7 @@ sub download_modularity_tests {
     assert_script_run 'dnf -y install python3-yaml', 180;
     assert_script_run 'curl -o /root/test.py https://pagure.io/fedora-qa/modularity_testing_scripts/raw/master/f/modular_functions.py';
     if ($whitelist eq 'whitelist') {
-	assert_script_run 'curl -o /root/whitelist https://pagure.io/fedora-qa/modularity_testing_scripts/raw/master/f/whitelist';
+    assert_script_run 'curl -o /root/whitelist https://pagure.io/fedora-qa/modularity_testing_scripts/raw/master/f/whitelist';
     }
     assert_script_run 'chmod 755 /root/test.py';
 }
@@ -1334,10 +1334,10 @@ sub check_prerelease {
         assert_screen "prerelease_note";
     }
     elsif ($prerelease == 0) {
-       # If the prerelease note is shown, where it should not be, die!
+        # If the prerelease note is shown, where it should not be, die!
-       if (check_screen "prerelease_note") {
+        if (check_screen "prerelease_note") {
             die "The PRERELEASE tag is shown, but it should NOT be.";
-       }
+        }
     }
 }
 

perllint.py

@@ -0,0 +1,110 @@
+#!/usr/bin/python3
+
+# Copyright Red Hat
+#
+# This file is part of os-autoinst-distri-fedora.
+#
+# os-autoinst-distri-fedora is free software; you can redistribute it
+# and/or modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation, either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# Author: Adam Williamson <awilliam@redhat.com>
+
+"""This is a simple linter for perl files in the repository. It fails
+if any line contains a tab, ends with whitespace characters (which
+includes lines containing *only* whitespace characters), or starts
+with a number of spaces that is not divisible by four. If run with
+--write, it will strip trailing whitespace (including turning lines
+that contain only whitespace into blank lines). It does not currently
+attempt to replace tabs or correct leading whitespace.
+"""
+
+import glob
+import os
+import sys
+
+
+def run(write):
+    """
+    Main run function.
+    """
+    exitcode = 0
+    scriptloc = os.path.dirname(os.path.realpath(__file__))
+    filenames = glob.glob(f"{scriptloc}/**/*.pm", recursive=True)
+    for filename in filenames:
+        if check_file(filename, write):
+            exitcode = 1
+    if exitcode == 0:
+        print("No errors found!")
+    sys.exit(exitcode)
+
+
+def check_file(filename, write):
+    """
+    Do the checks. Returns false if there is no failure, true if there
+    is a failure.
+    """
+    failed = False
+    with open(filename, "r", encoding="UTF-8") as readfh:
+        lines = readfh.readlines()
+        newlines = []
+        for (num, line) in enumerate(lines, 1):
+            # tab check
+            if "\t" in line:
+                print(f"Line {num} of file {filename} contains a tab!")
+                failed = True
+
+            # trailing whitespace check and replace
+            newline = line.rstrip("\n").rstrip(" \t") + "\n"
+            if newline != line:
+                print(f"Line {num} of file {filename} ends with a space or tab!")
+                failed = True
+                if write:
+                    # if we're writing changes, feed the *changed* line
+                    # to the next checks
+                    line = newline
+
+            # correct number of starting spaces check
+            spaces = 0
+            for char in line:
+                if char != " ":
+                    break
+                spaces += 1
+            if spaces > 0 and spaces % 4 != 0:
+                print(
+                    f"Line {num} of file {filename} starts with an invalid number of spaces!"
+                )
+                failed = True
+
+            # write the line back to newlines
+            if write:
+                newlines.append(line)
+
+    if write and newlines != lines:
+        with open(filename, "w", encoding="UTF-8") as writefh:
+            writefh.writelines(newlines)
+
+    return failed
+
+
+# Parse args and call run().
+if len(sys.argv) > 2:
+    sys.exit("Too many arguments!")
+
+if len(sys.argv) > 1:
+    if sys.argv[1] == "--write":
+        run(write=True)
+    else:
+        sys.exit("Invalid argument! Only valid argument is --write")
+
+# no args
+run(write=False)

tests/_do_install_and_reboot.pm

@@ -159,8 +159,8 @@ sub run {
         assert_script_run "chroot $mount grub2-mkconfig -o /boot/efi/EFI/fedora/grub.cfg";
     }
     if (grep {$_ eq 'abrt'} @actions) {
-         # Chroot in the newly installed system and switch on ABRT systemwide
+        # Chroot in the newly installed system and switch on ABRT systemwide
-         assert_script_run "chroot $mount abrt-auto-reporting 1";
+        assert_script_run "chroot $mount abrt-auto-reporting 1";
     }
     if (grep {$_ eq 'rootpw'} @actions) {
         my $root_password = get_var("ROOT_PASSWORD") || "weakpassword";

tests/_memcheck.pm

@@ -1,17 +1,17 @@
 use base "anacondatest";
- use strict;
+use strict;
- use testapi;
+use testapi;
 
- sub run {
+sub run {
-     my $self = shift;
+    my $self = shift;
-     $self->root_console();
+    $self->root_console();
-     upload_logs '/tmp/memory.dat';
+    upload_logs '/tmp/memory.dat';
- }
+}
 
- sub test_flags {
+sub test_flags {
-     return { fatal => 1 };
+    return { fatal => 1 };
- }
+}
 
- 1;
+1;
 
- # vim: set sw=4 et:
+# vim: set sw=4 et:

tests/_software_selection.pm

@@ -42,7 +42,7 @@ sub run {
     # select desired environment
     # go through the list 20 times at max (to prevent infinite loop when it's missing)
     for (my $i = 0; !check_screen("anaconda_".$packageset."_highlighted", 1) && $i < 20; $i++) {
-	       send_key "down";
+        send_key "down";
     }
 
     send_key "spc";

tests/applications/help/help.pm

@@ -8,31 +8,31 @@ use utils;
 # This subroutine opens a section, checks that its content
 # is listed and returns to the main page.
 sub visit_section {
-        my $section = shift;
+    my $section = shift;
-        send_key_until_needlematch("help_section_$section", "down", 40, 1 );
+    send_key_until_needlematch("help_section_$section", "down", 40, 1 );
-        click_lastmatch();
+    click_lastmatch();
-        assert_screen("help_section_content_$section");
+    assert_screen("help_section_content_$section");
-        assert_and_click("help_breadcrumbs_home");
+    assert_and_click("help_breadcrumbs_home");
-        assert_screen("help_main_screen");
+    assert_screen("help_main_screen");
 }
 
 sub run {
     my $self = shift;
 
-   	# Run the application
+    # Run the application
-        menu_launch_type("Help");
+    menu_launch_type("Help");
-        assert_screen("help_main_screen", timeout => 60);
+    assert_screen("help_main_screen", timeout => 60);
 
-   	# Let us click on Section to open it and check that there is content inside.
+    # Let us click on Section to open it and check that there is content inside.
-        visit_section("desktop");
+    visit_section("desktop");
-        visit_section("networking");
+    visit_section("networking");
-        visit_section("sound");
+    visit_section("sound");
-        visit_section("files");
+    visit_section("files");
-        visit_section("user");
+    visit_section("user");
-        visit_section("hardware");
+    visit_section("hardware");
-        visit_section("accessibility");
+    visit_section("accessibility");
-        visit_section("tipstricks");
+    visit_section("tipstricks");
-        visit_section("morehelp");
+    visit_section("morehelp");
 }
 
 sub test_flags {

tests/applications/nautilus/hidden_files.pm

@@ -6,56 +6,56 @@ use utils;
 # Show and unshow hidden files.
 
 sub run {
-	my $self = shift;
+    my $self = shift;
 
-	#  Enter the Documents directory.
+    #  Enter the Documents directory.
-	assert_and_click("nautilus_directory_documents", timeout => '30', button => 'left', mousehide => '1');
+    assert_and_click("nautilus_directory_documents", timeout => '30', button => 'left', mousehide => '1');
 
-	# Check that we are in the Documents directory.
+    # Check that we are in the Documents directory.
-	assert_screen("nautilus_documents_active", timeout => '30', no_wait => '0');
+    assert_screen("nautilus_documents_active", timeout => '30', no_wait => '0');
 
-	# Check if the hidden files are set to shown. If so, let's remember this status quo for further testing.
+    # Check if the hidden files are set to shown. If so, let's remember this status quo for further testing.
-	my $hidden_a = 0;
+    my $hidden_a = 0;
-	my $hidden_b = 0;
+    my $hidden_b = 0;
 
-	# Set the a variable to 1, if the hidden files are shown.
+    # Set the a variable to 1, if the hidden files are shown.
-	if (check_screen("nautilus_hidden_file_shown")) {
+    if (check_screen("nautilus_hidden_file_shown")) {
-	    $hidden_a = 1;
+        $hidden_a = 1;
-	}
+    }
 
-	# Now toggle the status of the invisible files and check that it is changed now
+    # Now toggle the status of the invisible files and check that it is changed now
-	# using the keyboard shortcut.
+    # using the keyboard shortcut.
-	send_key("ctrl-h");
+    send_key("ctrl-h");
-	wait_still_screen(2);
+    wait_still_screen(2);
 
-	# Now let us check again, if the invisible file is seen.
+    # Now let us check again, if the invisible file is seen.
-	if (check_screen("nautilus_hidden_file_shown")) {
+    if (check_screen("nautilus_hidden_file_shown")) {
-	   $hidden_b = 1;   	
+        $hidden_b = 1;
-	}
+    }
 
-	# Compare the results. They should differ from each other.
+    # Compare the results. They should differ from each other.
-	if ($hidden_a == $hidden_b) {
+    if ($hidden_a == $hidden_b) {
-	    die("The ctrl-h keyboard shortcut should have changed the status of invisible files, but the status has not been changed which indicates that the shortcut might not have worked correctly.");
+        die("The ctrl-h keyboard shortcut should have changed the status of invisible files, but the status has not been changed which indicates that the shortcut might not have worked correctly.");
-	}
+    }
 
-	# Now use the menu to change the status of the invisible files.
+    # Now use the menu to change the status of the invisible files.
-	assert_and_click("gnome_burger_menu");
+    assert_and_click("gnome_burger_menu");
-	wait_still_screen(2);
+    wait_still_screen(2);
-	assert_and_click("nautilus_toggle_hidden_files", timeout => '30', button => 'left', mousehide => '1');
+    assert_and_click("nautilus_toggle_hidden_files", timeout => '30', button => 'left', mousehide => '1');
-	wait_still_screen(2);
+    wait_still_screen(2);
 
-	# Check the current status of the invisible files.
+    # Check the current status of the invisible files.
-	if (check_screen("nautilus_hidden_file_shown")) {
+    if (check_screen("nautilus_hidden_file_shown")) {
-	    $hidden_b = 1;
+        $hidden_b = 1;
-	}
+    }
-	else {
+    else {
-	    $hidden_b = 0;
+        $hidden_b = 0;
-	}
+    }
 
-	# Compare the results. They should be the same if everything up til now has worked correctly.
+    # Compare the results. They should be the same if everything up til now has worked correctly.
-	if ($hidden_a != $hidden_b) {
+    if ($hidden_a != $hidden_b) {
-	   die("The menu item 'Show hidden files' should have changed the status of the invisible files, but the status has not changed which indicates that the menu item might not have worked correctly.");
+        die("The menu item 'Show hidden files' should have changed the status of the invisible files, but the status has not changed which indicates that the menu item might not have worked correctly.");
-	}
+    }
 }
 
 sub test_flags {

tests/applications/nautilus/side_panel.pm

@@ -16,7 +16,7 @@ sub run {
 
     my $side_panel = 0;
     if (check_screen("nautilus_side_panel")) {
-       	$side_panel = 1;
+        $side_panel = 1;
     }
 
     # Switch off the side panel
@@ -31,7 +31,7 @@ sub run {
 
     # Compare the results, they should be different.
     if ($off_panel == $side_panel) {
-       die("The side panel visibility should have been toggled and it is not. That indicates that the keyboard shortcut to toggle it does not work properly.");
+        die("The side panel visibility should have been toggled and it is not. That indicates that the keyboard shortcut to toggle it does not work properly.");
     }
 
     # Use menu to toggle the side panel back.
@@ -47,7 +47,7 @@ sub run {
 
     # Compare the results. If everything has been ok until now, the values should not be different.
     if ($off_panel != $side_panel) {
-       die("The side panel visibility should have been toggled back, but it has not been. That indicates that the menu item to toggle it does not work properly.");
+        die("The side panel visibility should have been toggled back, but it has not been. That indicates that the menu item to toggle it does not work properly.");
     }
 
 }

tests/disk_guided_empty.pm

@@ -18,16 +18,16 @@ sub run {
     if (get_var('TEST_UPDATES')){
         assert_screen "anaconda_install_destination_updates", 30;
     }
-   # Here the self identification test code is placed.
+    # Here the self identification test code is placed.
-   my $branched = get_var('VERSION');
+    my $branched = get_var('VERSION');
-   if ($identification eq 'true' or $branched ne "Rawhide") {
+    if ($identification eq 'true' or $branched ne "Rawhide") {
         check_top_bar(); # See utils.pm
         # we don't check version or pre-release because here those
         # texts appear on the banner which makes the needling
         # complex and fragile (banner is different between variants,
         # and has a gradient so for RTL languages the background color
         # differs; pre-release text is also translated)
-   }
+    }
 
     assert_and_click "anaconda_spoke_done";
 

tests/server_cockpit_updates.pm

@@ -28,10 +28,10 @@ sub run {
     # to deal with much more complicated workflow.
     #
     if (check_screen('cockpit_updates_security_install',1)) {
-       assert_and_click 'cockpit_updates_security_install';
+        assert_and_click 'cockpit_updates_security_install';
-       sleep 60; # wait until things are installed
+        sleep 60; # wait until things are installed
-       assert_and_click 'cockpit_updates_restart_ignore';
+        assert_and_click 'cockpit_updates_restart_ignore';
-       wait_still_screen 2;
+        wait_still_screen 2;
     }
 
 

tox.ini

@@ -14,6 +14,7 @@ deps =
 commands=
     python ./fifloader.py --clean templates.fif.json templates-updates.fif.json
     python ./check-needles.py
+    python ./perllint.py
     py.test unittests/
     py.test --cov-report term-missing --cov-report xml --cov fifloader unittests/
     diff-cover coverage.xml --fail-under=90 --compare-branch=origin/main