From 43e96e4c6a3261160e5133fbae9af33d4ea589c2 Mon Sep 17 00:00:00 2001 From: Mike Fleetwood Date: Mon, 8 May 2023 08:29:02 +0100 Subject: [PATCH] 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] 57983b9fc20c9367f9dd83a8301e903fced5329e 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 --- tests/test_BlockSpecial.cc | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_BlockSpecial.cc b/tests/test_BlockSpecial.cc index f4ee9b6c..d849415a 100644 --- a/tests/test_BlockSpecial.cc +++ b/tests/test_BlockSpecial.cc @@ -50,12 +50,15 @@ #include #include #include +#include +#include #include #include #include #include #include + namespace GParted { @@ -95,6 +98,19 @@ std::ostream& operator<<( std::ostream & out, const BlockSpecial & bs ) " Where: " << #b1 << " = " << (b1) << "\n" \ " 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 // /proc/partitions. static std::string get_block_name( unsigned want ) @@ -208,6 +224,7 @@ TEST( BlockSpecialTest, NamedBlockSpecialObjectPlainFileDuplicate ) TEST( BlockSpecialTest, NamedBlockSpecialObjectBlockDevice ) { 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). BlockSpecial::clear_cache(); @@ -219,6 +236,7 @@ TEST( BlockSpecialTest, NamedBlockSpecialObjectBlockDevice ) TEST( BlockSpecialTest, NamedBlockSpecialObjectBlockDeviceDuplicate ) { 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 // 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 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 // (name, major, minor). @@ -248,6 +268,7 @@ TEST( BlockSpecialTest, NamedBlockSpecialObjectBySymlinkMatches ) { std::string lname = get_link_name(); 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 // BlockSpecial objects with different names but the same major, minor pair. @@ -309,6 +330,7 @@ TEST( BlockSpecialTest, OperatorEqualsDifferentPlainFiles ) TEST( BlockSpecialTest, OperatorEqualsSameBlockDevices ) { 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. BlockSpecial::clear_cache(); @@ -320,6 +342,7 @@ TEST( BlockSpecialTest, OperatorEqualsSameBlockDevices ) TEST( BlockSpecialTest, OperatorEqualsEmptyObjectAndBlockDevice ) { std::string bname = get_block_name( 0 ); + SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname); // Test inequality of empty and named block device BlockSpecial objects. BlockSpecial::clear_cache(); @@ -331,6 +354,7 @@ TEST( BlockSpecialTest, OperatorEqualsEmptyObjectAndBlockDevice ) TEST( BlockSpecialTest, OperatorEqualsPlainFileAndBlockDevice ) { std::string bname = get_block_name( 0 ); + SKIP_IF_BLOCK_DEVICE_DOESNT_EXIST(bname); // Test inequality of plain file and block device BlockSpecial objects. BlockSpecial::clear_cache(); @@ -343,6 +367,8 @@ TEST( BlockSpecialTest, OperatorEqualsTwoDifferentBlockDevices ) { std::string bname1 = get_block_name( 0 ); 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. BlockSpecial::clear_cache(); @@ -406,6 +432,7 @@ TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndPlainFile ) TEST( BlockSpecialTest, OperatorLessThanSameBlockDevices ) { 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 // from the same name. @@ -418,6 +445,7 @@ TEST( BlockSpecialTest, OperatorLessThanSameBlockDevices ) TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndBlockDevice ) { 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. BlockSpecial::clear_cache(); @@ -429,6 +457,7 @@ TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndBlockDevice ) TEST( BlockSpecialTest, OperatorLessThanPlainFileAndBlockDevice ) { 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. BlockSpecial::clear_cache();