Also check links to block devices when skipping BlockSpecial unit tests (!113)

Fragment of a failed CI test job from a GiLab job runner which didn't
allow creation of block special devices looked like:

    $ tests/makedev.sh
    mknod -m 0660 /dev/sda b 8 0
    mknod: '/dev/sda': Operation not permitted
    chown: cannot access '/dev/sda': No such file or directory
    mknod -m 0660 /dev/sda1 b 8 1
    mknod: '/dev/sda1': Operation not permitted
    chown: cannot access '/dev/sda1': No such file or directory
    mkdir: created directory '/dev/disk'
    mkdir: created directory '/dev/disk/by-id/'
    '/dev/disk/by-id/gparted-sda' -> '/dev/sda'

test/makedev.sh attempted to create two block devices it wanted for
testing, but that failed with "Operation not permitted".  It then
created dangling symbolic link /dev/disk/by-id/gparted-sda -> /dev/sda;
gparted-sda pointed to a name which didn't exist.

Despite the previous commit testing and skipping every test where the
block device doesn't exist this unit test still failed:

    [ RUN      ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches
    test_BlockSpecial.cc:186: Failure
    Failed
    follow_link_name(): Failed to resolve symbolic link '/dev/disk/by-id/gparted-sda'
    test_BlockSpecial.cc:271: Skip test.  Block device '' does not exist
    [  FAILED  ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches (0 ms)

The unit test called get_link_name() which read the directory
/dev/disk/by-id and found symbolic link gparted-sda.  It then called
follow_link_name() passing /dev/disk/by-id/gparted-sda which used
realpath(3) to get the canonicalised absolute pathname, which includes
following links.  But as gparted-sda pointed to a non-existent file it
failed and reported message "Failed to resolve symbolic link ...".  Then
after that the unit test skips the non-existent block device, but the
test has already failed at that point.

Fix the unit test by also checking the symbolic link points to an
existing block device before calling follow_link_name() on it.  This
works because SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST() uses stat(3), which
follows symbolic links, in it's verification.

Also put SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST() immediately after each
initialisation of a block device name for some sort of consistency with
it's need in this fixed NamedBlockSpecialObjectBySymlinkMatches unit
test.

Closed !113 - Fix occasional GitLab CI test jobs failures on
              BlockSpecial unit tests
This commit is contained in:
Mike Fleetwood 2023-05-13 13:43:50 +01:00 committed by Curtis Gedak
parent 43e96e4c6a
commit cc4687a2aa

View file

@ -251,8 +251,8 @@ TEST( BlockSpecialTest, NamedBlockSpecialObjectBlockDeviceDuplicate )
TEST( BlockSpecialTest, TwoNamedBlockSpecialObjectBlockDevices )
{
std::string bname1 = get_block_name( 0 );
std::string bname2 = get_block_name( 1 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname1);
std::string bname2 = get_block_name( 1 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname2);
// Test that two different named block devices produce different
@ -267,6 +267,7 @@ TEST( BlockSpecialTest, TwoNamedBlockSpecialObjectBlockDevices )
TEST( BlockSpecialTest, NamedBlockSpecialObjectBySymlinkMatches )
{
std::string lname = get_link_name();
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(lname);
std::string bname = follow_link_name( lname );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname);
@ -366,8 +367,8 @@ TEST( BlockSpecialTest, OperatorEqualsPlainFileAndBlockDevice )
TEST( BlockSpecialTest, OperatorEqualsTwoDifferentBlockDevices )
{
std::string bname1 = get_block_name( 0 );
std::string bname2 = get_block_name( 1 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname1);
std::string bname2 = get_block_name( 1 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname2);
// Test inequality of two different named block device BlockSpecial objects.