Test coverage and refactoring in preparation for steamrt/tasks#595
Hopefully all of this is uncontroversial. Anything that attracts review comments should probably be split out into its own MR.
-
utils: Factor out _srt_string_ends_with_len()
If the length is already known, this avoids recalculating it (and if the suffix contains embedded NULs, this makes it possible to check for).
-
tests: Add some missing coverage
We never explicitly tested the case where _srt_string_ends_with() is looking for a suffix longer than the string itself.
-
tests: Add missing test coverage for what happens without PvPerArchDirs
If we are unable to discover
$LIB
and$PLATFORM
, or if they do not fit any of the patterns we expect, then we might have to skip reorganising the LD_PRELOAD and LD_AUDIT modules. Test this, to make sure refactoring does not make it regress. -
tests: Ensure we have better coverage if built with --werror
Release builds are not done with --werror, so we will gracefully skip various parts of the build if we are running on a particularly strange platform (where /dev/pts isn't a directory, /dev/stderr isn't a symlink, or we are unable to expand
${LIB}
and${PLATFORM}
). But for our official CI and individual developers' builds, we expect the build to be done on a platform that makes sense, so we can be more strict. -
tests: When building with --werror, make assertions about /etc/os-release
We can expect and assume that on our official CI or on developer machines, we'll be running in a reasonable environment.
-
tests: Future-proof PvAdverbPreloadModule initializers
This will let the test continue to pass without code changes when we add new struct members, as long as zero/NULL is a valid default value for those members, reducing the diffstat for refactoring in this area.
-
pv-wrap: Remove redundant reinitialization of SrtSystemInfo
Instantiating this can't fail, and if it somehow does, trying it again is unlikely to help.
-
build-aux: Add convenience files to cross-compile for arm64
This is a useful way to check that non-x86 builds aren't regressing, without actually needing a non-x86 device.
Not all tests will pass in this configuration because qemu-user is not a perfect emulation of a real aarch64 machine (for example PR_GET_CHILD_SUBREAPER is unimplemented), but it's better than nothing. In particular, the adverb-preload and wrap-setup tests pass, and those are the ones I'm altering for steamrt/tasks#595.
-
pressure-vessel: Avoid looping through zero-length array of emulator hosts
If compiled for a non-x86 architecture, we don't expect to be emulated by some foreign architecture with a mechanism like FEX or qemu-user, so PV_N_SUPPORTED_ARCHITECTURES_AS_EMULATOR_HOST is zero. In this case don't try to allocate and loop over an array of length 0. This also avoids gcc warnings with -Wtype-limits, "comparison of unsigned expression in ‘< 0’ is always false".
-
tests: Combine expected test results for x86_64 and non-x86
There are three differences between our handling of x86 and non-x86 here:
- On x86, x86_64 is always treated as the primary architecture, with i386 as secondary (even if pressure-vessel itself is compiled as an i386 binary). On other platforms, the native platform is primary.
- On x86, i386 is available as a secondary architecture. On other platforms, there its no secondary architecture.
- We have different mock expansions for
${LIB}
,${PLATFORM}
and the multiarch tuple on x86 and non-x86.
By abstracting the "primary architecture", we can avoid having lines of test code that are only run on non-x86 platforms, which in practice means they are not routinely run at all. This makes them less likely to regress.
-
pv-wrap: Document PV_APPEND_PRELOAD_FLAGS_IN_UNIT_TESTS
Fixes: 08751fff "tests: Assert that modules with ${LIB} or ${PLATFORM} are handled"
-
pv-wrap: Have test coverage for the single-architecture case
Until now, the case where only one architecture is supported has only been tested if pressure-vessel was built for a non-x86 architecture such as ARM, which is rarely (if ever) done.
We can get coverage for this case by making pv-wrap behave as if only the first architecture was supported, as a runtime rather than compile-time decision: this means that an ordinary x86_64 build can exercise both the x86 and non-x86 code paths.
This is important because steamrt/tasks#595 will add new code that differs according to whether there is only a single architecture, or more than one.
In the test, this means we need to be able to mark which of the expected paths are expected on i386 only, and which ones are expected in general. Instead of the verbosity that would result from turning all of our test data into structs, I've done this with an ad-hoc mini-language: the ones that are only expected on i386 are prefixed with
i386:
.We also might conceivably want to make use of something similar to PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE in production in future, if we start providing legacy-free containers that only support running x86_64 games.
Merge request reports
Activity
added Awaiting Review Tech debt pressure-vessel labels
requested review from @denittis
assigned to @smcv
removed Awaiting Review label
added To Test label
removed To Test label
mentioned in commit b7943f7a