Skip BlockSpecial unit tests when devices don't exist, for CI test images (!113)

Since November 2022 test_BlockSpecial has been occasionally failing in
GNOME GitLab Docker CI test jobs like this:

    [ RUN      ] BlockSpecialTest.NamedBlockSpecialObjectBlockDevice
    test_BlockSpecial.cc:216: Failure
    Value of: bs.m_major > 0 || bs.m_minor > 0
      Actual: false
    Expected: true
    [  FAILED  ] BlockSpecialTest.NamedBlockSpecialObjectBlockDevice (0 ms)
    ...
    [ RUN      ] BlockSpecialTest.TwoNamedBlockSpecialObjectBlockDevices
    test_BlockSpecial.cc:244: Failure
    Value of: bs1.m_major != bs2.m_major || bs1.m_minor != bs2.m_minor
      Actual: false
    Expected: true
    [  FAILED  ] BlockSpecialTest.TwoNamedBlockSpecialObjectBlockDevices (0 ms)
    [ RUN      ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches
    test_BlockSpecial.cc:170: Failure
    Failed
    follow_link_name(): Failed to resolve symbolic link '/dev/disk/by-id/gparted-sda'
    [  FAILED  ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches (0 ms)
    ...
     3 FAILED TESTS
    FAIL test_BlockSpecial (exit status: 1)

As identified previously [1] the Docker CI images no longer have any
block devices in /dev.  test/makedev.sh script was added to create block
devices test_BlockSpecial needs for it's testing.  Now a subset of the
GNOME GitLab job runners additionally prevent creation of block special
device nodes.  test/makedev.sh reports this:

    $ 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

Alternative rejected solutions:

1.  Use fakeroot [2].  Package is available for the 3 distributions used
    in CI jobs.  Does fake stat() call.  Works when run like this in the
    CI test jobs:
        fakeroot -s test/fakeroot.env tests/makedev.sh
        fakeroot -i test/fakeroot.env make check
        fakeroot -i test/fakeroot.env make distcheck
    But if you run fakeroot ... make check on our development machines
    as a non-root user it causes the test_SupportedFileSystems unit
    tests which use losetup to fail.  This is because
    test_SupportedFileSystems thinks it's root inside the fakeroot
    environment but fakeroot doesn't fake enough for losetup to work.
    This makes running tests in the GitLab CI jobs different from how we
    would have to run them on our development machines.  Prefer not to
    do that.

2.  Use GNU ld --wrap [3] to call our own __wrap_stat() allowing
    test_BlockSpecial to provide mocked results to the stat() call in
    constructor BlockSpecial::BlockSpecial().  This works with
    GNU C Library >= 2.33, released 01-Feb-2021, and musl libc,
    therefore it works on CI tested distributions Ubuntu LTS >= 22.04
    and Alpine Linux respectively.  However this fails on earlier glibc
    releases, so will fail on CentOS 7 CI image, as the compiler emits a
    call to __xstat() rather than stat().  This is something to do with
    how glibc's /usr/include/sys/stat.h supported multiple versions of
    stat().  Don't use this as it's doesn't work everywhere.

    Additional useful implementation hints.  [4][5]

Choose to fix by just skipping unit tests which need block special names
to exist in the file system, but don't exist.  This is the same
technique that test_SupportedFileSystems uses.  So tests/makedev.sh
creates block devices if it can in the GNOME GitLab CI test images [1]
and now if that fails the individual unit tests are skipped.

[1] 57983b9fc2
    Create block special devices needed by test_BlockSpecial in GitLab
    CI jobs (!59)

[2] FakeRoot
    https://wiki.debian.org/FakeRoot

[3] ld(1) - Linux manual page
    https://man7.org/linux/man-pages/man1/ld.1.html
        "--wrap=symbol
        Use a wrapper function for symbol.  Any undefined reference to
        symbol will be resolved to "__wrap_symbol".  Any undefined
        reference to "__real_symbol" will be resolved to symbol.

        This can be used to provide a wrapper for a system function.
        The wrapper function should be called "__wrap_symbol".  If it
        wishes to call the system function, it should call
        "__real_symbol".
        ...
        "

[4] gcc: error: unrecognized option --wrap
    https://stackoverflow.com/questions/33278164/gcc-error-unrecognized-option-wrap

[5] C++ ld linker --wrap option does not work for internal function calls
    https://stackoverflow.com/questions/44464961/c-ld-linker-wrap-option-does-not-work-for-internal-function-calls

Closed !113 - Fix occasional GitLab CI test jobs failures on
              BlockSpecial unit tests
This commit is contained in:
Mike Fleetwood 2023-05-08 08:29:02 +01:00 committed by Curtis Gedak
parent 011317b23f
commit 43e96e4c6a
1 changed files with 29 additions and 0 deletions

View File

@ -50,12 +50,15 @@
#include <fstream> #include <fstream>
#include <stdio.h> #include <stdio.h>
#include <sys/types.h> #include <sys/types.h>
#include <sys/stat.h>
#include <errno.h>
#include <dirent.h> #include <dirent.h>
#include <limits.h> #include <limits.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <glibmm/ustring.h> #include <glibmm/ustring.h>
namespace GParted namespace GParted
{ {
@ -95,6 +98,19 @@ std::ostream& operator<<( std::ostream & out, const BlockSpecial & bs )
" Where: " << #b1 << " = " << (b1) << "\n" \ " Where: " << #b1 << " = " << (b1) << "\n" \
" And: " << #b2 << " = " << (b2); " And: " << #b2 << " = " << (b2);
#define SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(name) \
{ \
struct stat sb; \
if (stat(name.c_str(), &sb) != 0 && errno == ENOENT) \
{ \
std::cout << __FILE__ << ":" << __LINE__ << ": Skip test. Block " \
<< "device '" << name << "' does not exist" << std::endl; \
return; \
} \
}
// Return block device names numbered 0 upwards like "/dev/sda" by reading entries from // Return block device names numbered 0 upwards like "/dev/sda" by reading entries from
// /proc/partitions. // /proc/partitions.
static std::string get_block_name( unsigned want ) static std::string get_block_name( unsigned want )
@ -208,6 +224,7 @@ TEST( BlockSpecialTest, NamedBlockSpecialObjectPlainFileDuplicate )
TEST( BlockSpecialTest, NamedBlockSpecialObjectBlockDevice ) TEST( BlockSpecialTest, NamedBlockSpecialObjectBlockDevice )
{ {
std::string bname = get_block_name( 0 ); std::string bname = get_block_name( 0 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname);
// Test any block special name produces BlockSpecial object (name, major, minor). // Test any block special name produces BlockSpecial object (name, major, minor).
BlockSpecial::clear_cache(); BlockSpecial::clear_cache();
@ -219,6 +236,7 @@ TEST( BlockSpecialTest, NamedBlockSpecialObjectBlockDevice )
TEST( BlockSpecialTest, NamedBlockSpecialObjectBlockDeviceDuplicate ) TEST( BlockSpecialTest, NamedBlockSpecialObjectBlockDeviceDuplicate )
{ {
std::string bname = get_block_name( 0 ); std::string bname = get_block_name( 0 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname);
// Test that a second object with the same name of a block device produces the // Test that a second object with the same name of a block device produces the
// same (name, major, minor). Checking internal BlockSpecial caching again. // same (name, major, minor). Checking internal BlockSpecial caching again.
@ -234,6 +252,8 @@ TEST( BlockSpecialTest, TwoNamedBlockSpecialObjectBlockDevices )
{ {
std::string bname1 = get_block_name( 0 ); std::string bname1 = get_block_name( 0 );
std::string bname2 = get_block_name( 1 ); std::string bname2 = get_block_name( 1 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname1);
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname2);
// Test that two different named block devices produce different // Test that two different named block devices produce different
// (name, major, minor). // (name, major, minor).
@ -248,6 +268,7 @@ TEST( BlockSpecialTest, NamedBlockSpecialObjectBySymlinkMatches )
{ {
std::string lname = get_link_name(); std::string lname = get_link_name();
std::string bname = follow_link_name( lname ); std::string bname = follow_link_name( lname );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname);
// Test that a symbolic link to a block device and the named block device produce // Test that a symbolic link to a block device and the named block device produce
// BlockSpecial objects with different names but the same major, minor pair. // BlockSpecial objects with different names but the same major, minor pair.
@ -309,6 +330,7 @@ TEST( BlockSpecialTest, OperatorEqualsDifferentPlainFiles )
TEST( BlockSpecialTest, OperatorEqualsSameBlockDevices ) TEST( BlockSpecialTest, OperatorEqualsSameBlockDevices )
{ {
std::string bname = get_block_name( 0 ); std::string bname = get_block_name( 0 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname);
// Test equality of two BlockSpecial objects using the same named block device. // Test equality of two BlockSpecial objects using the same named block device.
BlockSpecial::clear_cache(); BlockSpecial::clear_cache();
@ -320,6 +342,7 @@ TEST( BlockSpecialTest, OperatorEqualsSameBlockDevices )
TEST( BlockSpecialTest, OperatorEqualsEmptyObjectAndBlockDevice ) TEST( BlockSpecialTest, OperatorEqualsEmptyObjectAndBlockDevice )
{ {
std::string bname = get_block_name( 0 ); std::string bname = get_block_name( 0 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname);
// Test inequality of empty and named block device BlockSpecial objects. // Test inequality of empty and named block device BlockSpecial objects.
BlockSpecial::clear_cache(); BlockSpecial::clear_cache();
@ -331,6 +354,7 @@ TEST( BlockSpecialTest, OperatorEqualsEmptyObjectAndBlockDevice )
TEST( BlockSpecialTest, OperatorEqualsPlainFileAndBlockDevice ) TEST( BlockSpecialTest, OperatorEqualsPlainFileAndBlockDevice )
{ {
std::string bname = get_block_name( 0 ); std::string bname = get_block_name( 0 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname);
// Test inequality of plain file and block device BlockSpecial objects. // Test inequality of plain file and block device BlockSpecial objects.
BlockSpecial::clear_cache(); BlockSpecial::clear_cache();
@ -343,6 +367,8 @@ TEST( BlockSpecialTest, OperatorEqualsTwoDifferentBlockDevices )
{ {
std::string bname1 = get_block_name( 0 ); std::string bname1 = get_block_name( 0 );
std::string bname2 = get_block_name( 1 ); std::string bname2 = get_block_name( 1 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname1);
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname2);
// Test inequality of two different named block device BlockSpecial objects. // Test inequality of two different named block device BlockSpecial objects.
BlockSpecial::clear_cache(); BlockSpecial::clear_cache();
@ -406,6 +432,7 @@ TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndPlainFile )
TEST( BlockSpecialTest, OperatorLessThanSameBlockDevices ) TEST( BlockSpecialTest, OperatorLessThanSameBlockDevices )
{ {
std::string bname = get_block_name( 0 ); std::string bname = get_block_name( 0 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname);
// Test one name block device BlockSpecial object is not ordered before another // Test one name block device BlockSpecial object is not ordered before another
// from the same name. // from the same name.
@ -418,6 +445,7 @@ TEST( BlockSpecialTest, OperatorLessThanSameBlockDevices )
TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndBlockDevice ) TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndBlockDevice )
{ {
std::string bname = get_block_name( 0 ); std::string bname = get_block_name( 0 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname);
// Test empty BlockSpecial object with name "" is before any block device. // Test empty BlockSpecial object with name "" is before any block device.
BlockSpecial::clear_cache(); BlockSpecial::clear_cache();
@ -429,6 +457,7 @@ TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndBlockDevice )
TEST( BlockSpecialTest, OperatorLessThanPlainFileAndBlockDevice ) TEST( BlockSpecialTest, OperatorLessThanPlainFileAndBlockDevice )
{ {
std::string bname = get_block_name( 0 ); std::string bname = get_block_name( 0 );
SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname);
// Test one plain file BlockSpecial object is ordered before any block device. // Test one plain file BlockSpecial object is ordered before any block device.
BlockSpecial::clear_cache(); BlockSpecial::clear_cache();