Categories
notest Perl PHP

Creating reproducible tests in pull requests so reviewers can know the code is properly working when your codebase lacks unit tests.

Everyone who has worked before with unit testing will know that pull request based testing is highly inefficient on the sense tests are going to be lost once the pull request is approved. But meanwhile automated testing is implemented in a legacy project code has still to be done to implement new features or bugfixes.

That is where the guideline I am going to expose has it’s niche, trying to get the job done while preserving a sane behaviour in the code.

This guide assumes a web based project, some parts may have to be adapted on discrection for other use cases.

General Guidelines.

  • Avoid high level instructions that can be written as commands.

It is usually the best to avoid saying the reviewers to do something that they may have to investigate when you can write some fancy command that does it. Example:

Bad example:

“Block the black haired users from having a avatar.”

Good example:

“Execute the following sql query to block the black haired users from having a avatar.”

update set avatar_blocked=1 from users where hair = 'black';

Other bad example:

Remove lines from 500-550 from lib/Users/BlockAvatar.php since they attempt to connect to a external ftp and are going to generate an error and set $got_external_csv to 0.

Good example:

Remove lines from 500-550 from lib/Users/BlockAvatar.php since they attempt to connect to a external ftp and are going to generate an error with this command and set $got_external_csv to 0:

perl <( cat << 'EOF'
use 5.30.0;
my $i = 0;
while (<>) {
    $i++; 
    next if $i >= 500 && $i <= 550;
    say << 'END_OF_SAY' if $i == 551;
        # Temporal fix to avoid ftp connections in testing.
        $got_external_csv = 0;
END_OF_SAY
    print;
}
EOF
) lib/Users/BlockAvatar.php > block_avatar_tmp.php
cp block_avatar_tmp.php lib/Users/BlockAvatar.php
rm block_avatar_tmp.php
  • Avoid to write a database query as a bash command and write the query directly so everyone can use whatever database client they find more comfortable with.

Bad example:

echo 'select * from users' | mysql

Good example:

select * from users;
  • Avoid to write operations in the webpage that users are supposed to do as bash commands and instead send the reviewers to do those operations unless it is really needed.

This is mainly because two reasons, if you bindly copy as Posix Curl a Firefox request you have chances to collide against csfr tokens or leak your authentication cookies, not a good deal, also you have chances that if you broke something in frontend in your changes it gets unnoticed in the pull request.

Bad example:

“Delete the Luffy user”

curl -X DELETE www.myweb.com/api/user/luffy

Good example:

Go to https://www.myweb.com/admin/manage_user?user=luffy and press delete this user.

  • Include screenshots of GUI steps if possible indicating where should be reviewer interact with the webpage and how.

Those screenshots should not be an alternative against text description but a complement to avoid blind reviewer discrimination, it should be a help for people with visual minds, not a diversity killer.