Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Fix tools::is_hdd unit tests #9716

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iamamyth
Copy link

Correct the unit tests for tools::is_hdd to avoid making assumptions about the configuration of a particular device based solely on the value of the GLIBC preprocessor flag. Instead, rely on the test invoker to provide paths for devices of specific types via the process environment, thereby avoiding faulty assumptions and improving the specificity of test assertions. To ensure appropriate devices exist, add a script, tests/create_test_disks.sh, which configures loopback devices mirroring relevant configurations.

@iamamyth
Copy link
Author

Obsoletes #9712.

Correct the unit tests for tools::is_hdd to avoid making assumptions
about the configuration of a particular device based solely on the
value of the __GLIBC__ preprocessor flag. Instead, rely on the
test invoker to provide paths for devices of specific types via
the process environment, thereby avoiding faulty assumptions and
improving the specificity of test assertions. To ensure appropriate
devices exist, add a script, tests/create_test_disks.sh, which
configures loopback devices mirroring relevant configurations.

#ifndef GTEST_SKIP
#include <iostream>
#define SKIP_TEST(reason) do {std::cerr << "Skipping test: " << reason << std::endl; return;} while(0)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced this macro because GTEST_SKIP doesn't exist in fairly old versions of gtest (e.g. the vendored one, as well as the default installs on Debian 10 and Ubuntu 20), but should be available on every build in the near future, at which point I'll drop the macro.

Copy link
Author

@iamamyth iamamyth Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to the PR which introduced GTEST_SKIP: google/googletest#1544. As best as I can tell, it landed in the 1.10.0 release, dated 2019-10-03.

@iamamyth
Copy link
Author

iamamyth commented Jan 18, 2025

Test failure is one of the known flaky tests (node_server.race_condition), unrelated to this commit.

Copy link
Collaborator

@0xFFFC0000 0xFFFC0000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants