Skip to content

test: address five reviewer nits on tst_settings and tst_util #1502

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.

Sign up for GitHub

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

Merged
annejan merged 1 commit into from
May 17, 2026
Merged

test: address five reviewer nits on tst_settings and tst_util #1502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions tests/auto/settings/tst_settings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,9 @@ void tst_settings::setAndGetGeometry() {

void tst_settings::getPassStore() {
QString store = QtPassSettings::getPassStore();
QVERIFY2(store.isEmpty() || store.startsWith("/") || store.contains("/"),
"Pass store should be empty or a plausible path");
const bool plausiblePath = store.isEmpty() || QDir::isAbsolutePath(store) ||
store.contains('/') || store.contains('\\');
QVERIFY2(plausiblePath, "Pass store should be empty or a plausible path");
}

void tst_settings::setAndGetPassStore() {
Expand Down Expand Up @@ -512,18 +513,21 @@ void tst_settings::setAndGetMultipleProfiles() {
QVERIFY(readProfiles.contains("profile2"));
QVERIFY(readProfiles["profile1"].contains("path"));
QVERIFY(readProfiles["profile2"].contains("path"));
// Verify new git options are in profile (issue #112)
QVERIFY(readProfiles["profile1"].contains("useGit"));
QVERIFY(readProfiles["profile1"].contains("autoPush"));
QVERIFY(readProfiles["profile1"].contains("autoPull"));
// Verify git options default to empty (unset) - compare QString directly
// instead of toInt() which treats "true" as 0
QVERIFY2(readProfiles["profile1"].value("useGit").isEmpty(),
"useGit should default to empty");
QVERIFY2(readProfiles["profile1"].value("autoPush").isEmpty(),
"autoPush should default to empty");
QVERIFY2(readProfiles["profile1"].value("autoPull").isEmpty(),
"autoPull should default to empty");
// Verify new git options are in all profiles (issue #112)
const QStringList profileNames = {"profile1", "profile2"};
for (const QString &profileName : profileNames) {
QVERIFY(readProfiles[profileName].contains("useGit"));
QVERIFY(readProfiles[profileName].contains("autoPush"));
QVERIFY(readProfiles[profileName].contains("autoPull"));
// Verify git options default to empty (unset) - compare QString directly
// instead of toInt() which treats "true" as 0
QVERIFY2(readProfiles[profileName].value("useGit").isEmpty(),
"useGit should default to empty");
QVERIFY2(readProfiles[profileName].value("autoPush").isEmpty(),
"autoPush should default to empty");
QVERIFY2(readProfiles[profileName].value("autoPull").isEmpty(),
"autoPull should default to empty");
}
}

void tst_settings::profileGitOptions() {
Expand Down
36 changes: 16 additions & 20 deletions tests/auto/util/tst_util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ static constexpr int DISTRIBUTION_MAX_PERCENT = 120;
static constexpr int PERCENT_BASE = 100;
static constexpr int RANDOMNESS_TEST_SAMPLE_COUNT = 200;
static constexpr int RANDOMNESS_TEST_PASSWORD_LENGTH = 32;
// Permissive chi-square cutoff for df=9: set above the p=0.995 critical
// value (~23.59) to reduce false test failures while still catching
// meaningful distribution bias.
static constexpr double CHI_SQUARE_PERMISSIVE_THRESHOLD_DF9 = 30.0;

/**
Expand Down Expand Up @@ -1221,19 +1224,12 @@ void tst_util::isValidKeyIdWithEmail() {
}

void tst_util::isValidKeyIdInvalid() {
// NOTE: 40 is intentional and derived from the current Util::isValidKeyId
// implementation detail: its key-id regex accepts 8-40 hexadecimal
// characters (optionally with a 0x prefix). This test verifies that a
// value just above that upper bound is rejected.
//
// Keep boundary expectations aligned with production validation rules.
// This test checks that one character above the configured max is rejected.
// If Util::isValidKeyId's accepted range changes, update this constant.
constexpr int ASSUMED_MAX_KEY_ID_LENGTH = 40;
constexpr int TOO_LONG_KEY_ID_LENGTH = ASSUMED_MAX_KEY_ID_LENGTH + 1;
// Boundary check: one character above the currently accepted maximum
// hexadecimal key-id length should be rejected.
constexpr int kTooLongKeyIdLength = 41; // current max (40) + 1
QVERIFY(!Util::isValidKeyId(""));
QVERIFY(!Util::isValidKeyId("short"));
QVERIFY(!Util::isValidKeyId(QString(TOO_LONG_KEY_ID_LENGTH, 'a')));
QVERIFY(!Util::isValidKeyId(QString(kTooLongKeyIdLength, 'a')));
QVERIFY(!Util::isValidKeyId("invalidchars!"));
QVERIFY(!Util::isValidKeyId("space in key"));
}
Expand Down Expand Up @@ -2080,7 +2076,7 @@ void tst_util::grepImitatePassInvalidRegexEmitsEmpty() {
// RAII helper restores SSH_AUTH_SOCK + the override setting around each test
// so we don't pollute the developer's environment or each other's state.
// ---------------------------------------------------------------------------
namespace {
namespace testutils {
class SshAuthSockGuard {
public:
SshAuthSockGuard()
Expand All @@ -2103,13 +2099,13 @@ class SshAuthSockGuard {
QByteArray prevEnv_;
QString prevOverride_;
};
} // namespace
} // namespace testutils

/**
* @brief getter/setter roundtrip for sshAuthSockOverride.
*/
void tst_util::sshAuthSockOverrideRoundtrip() {
SshAuthSockGuard guard;
testutils::SshAuthSockGuard guard;
const QString sentinel =
QStringLiteral("/tmp/qtpass-test-sock-") + QUuid::createUuid().toString();
QtPassSettings::setSshAuthSockOverride(sentinel);
Expand All @@ -2122,7 +2118,7 @@ void tst_util::sshAuthSockOverrideRoundtrip() {
* @brief default value of getSshAuthSockOverride is empty.
*/
void tst_util::sshAuthSockOverrideEmptyByDefault() {
SshAuthSockGuard guard;
testutils::SshAuthSockGuard guard;
QtPassSettings::setSshAuthSockOverride(QString{});
QCOMPARE(QtPassSettings::getSshAuthSockOverride(), QString{});
}
Expand All @@ -2132,7 +2128,7 @@ void tst_util::sshAuthSockOverrideEmptyByDefault() {
* already set.
*/
void tst_util::initialiseSshAuthSockHonoursExistingEnv() {
SshAuthSockGuard guard;
testutils::SshAuthSockGuard guard;
const QByteArray existing("/tmp/qtpass-existing-sock");
qputenv("SSH_AUTH_SOCK", existing);
// Even if an override is configured, the existing env wins.
Expand All @@ -2147,7 +2143,7 @@ void tst_util::initialiseSshAuthSockHonoursExistingEnv() {
* unset.
*/
void tst_util::initialiseSshAuthSockUsesOverride() {
SshAuthSockGuard guard;
testutils::SshAuthSockGuard guard;
qunsetenv("SSH_AUTH_SOCK");
const QString override = QStringLiteral("/tmp/qtpass-override-sock");
QtPassSettings::setSshAuthSockOverride(override);
Expand All @@ -2162,7 +2158,7 @@ void tst_util::initialiseSshAuthSockUsesOverride() {
* the resulting state is consistent.
*/
void tst_util::initialiseSshAuthSockNoOverrideNoEnvProbes() {
SshAuthSockGuard guard;
testutils::SshAuthSockGuard guard;
qunsetenv("SSH_AUTH_SOCK");
QtPassSettings::setSshAuthSockOverride(QString{});
Util::initialiseSshAuthSock();
Expand All @@ -2184,7 +2180,7 @@ void tst_util::initialiseSshAuthSockNoOverrideNoEnvProbes() {
* the user starts their agent after launching QtPass).
*/
void tst_util::initialiseSshAuthSockOverrideSkipsAgentValidation() {
SshAuthSockGuard guard;
testutils::SshAuthSockGuard guard;
qunsetenv("SSH_AUTH_SOCK");
// A path that almost certainly has no listener — validation would fail.
// Override should win regardless.
Expand All @@ -2201,7 +2197,7 @@ void tst_util::initialiseSshAuthSockOverrideSkipsAgentValidation() {
* it should fall through to the auto-probe.
*/
void tst_util::initialiseSshAuthSockEmptyOverrideFallsThrough() {
SshAuthSockGuard guard;
testutils::SshAuthSockGuard guard;
qunsetenv("SSH_AUTH_SOCK");
QtPassSettings::setSshAuthSockOverride(QString{});
Util::initialiseSshAuthSock();
Expand Down
Loading