Skip to content

test: improve randomness and distribution tests#1010

Merged
annejan merged 1 commit into
mainfrom
fix/test-improvements
Apr 15, 2026
Merged

test: improve randomness and distribution tests#1010
annejan merged 1 commit into
mainfrom
fix/test-improvements

Conversation

@nogeenharrie
Copy link
Copy Markdown
Contributor

@nogeenharrie nogeenharrie commented Apr 15, 2026

User description

Summary

  • Add randomness sanity check to generatePassword test - verifies multiple calls produce different passwords
  • Add statistical distribution check to boundedRandom test - verifies counts are within expected tolerance

Testing

All tests pass:

  • generateRandomPassword
  • boundedRandom

CodeAnt-AI Description

Strengthen password randomness checks

What Changed

  • Repeated password generation is now checked to ensure it can produce different results, which helps catch broken randomness
  • Digit output is now tested for a more even spread across all digits, not just that each digit appears at least once

Impact

✅ Fewer false passes in randomness tests
✅ Earlier detection of weak password generation
✅ More reliable test coverage for random output

🔄 Retrigger CodeAnt AI Review

Details

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

  • Tests
    • Strengthened password-generation tests to verify output length, ensure every character comes from the chosen charset, and confirm outputs vary across multiple runs (non-determinism).
    • Replaced weaker distribution checks with a statistical chi-square uniformity test for digit frequencies, and report the computed statistic and threshold on failure to improve randomness validation.

@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai Bot commented Apr 15, 2026

CodeAnt AI is reviewing your PR.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 15, 2026

📝 Walkthrough

Walkthrough

Tests were tightened for randomness and charset correctness: generateRandomPassword() now validates a 32-char password with charset "abcd", ensures every character belongs to that charset, and ensures at least one of up to 20 subsequent generations differs; boundedRandom() now computes a chi-square over digits 0–9 and asserts it is below 25.0.

Changes

Cohort / File(s) Summary
Test Enhancements
tests/auto/util/tst_util.cpp
Added length and per-character charset checks for generateRandomPassword(32, "abcd"), plus up to 20 additional generations to assert non-identity with the first sample; enhanced boundedRandom() assertions by computing a chi-square statistic over digit counts (0–9) and asserting it is below a fixed critical threshold (25.0), reporting the computed chi-square and threshold on failure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 With "abcd" clutched softly in my paw,

I hop through samples, checking every law.
Twenty tries later, one must find new art—
Digits dance, chi-square plays its part.
Tests sing softly: randomized from the start.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: improving randomness and distribution tests for password generation and bounded random functions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/test-improvements

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai codeant-ai Bot added the size:M This PR changes 30-99 lines, ignoring generated files label Apr 15, 2026
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai Bot commented Apr 15, 2026

CodeAnt AI finished reviewing your PR.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/auto/util/tst_util.cpp`:
- Around line 597-609: Replace the current per-digit ±50% tolerance check with a
chi-square goodness-of-fit test: compute expected = iterations / 10 and chi2 =
sum over i of (counts[i] - expected)^2 / expected, then assert chi2 is below the
critical value for 9 degrees of freedom (e.g. ~16.92 for alpha=0.05) using
QVERIFY2; reference the existing variables counts, iterations, expectedPerDigit
(or expected), count and the QVERIFY2 assertion so the message reports the
computed chi2 and the critical threshold on failure.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6d145c82-c057-475b-b1e7-1d5a7bd0e1c0

📥 Commits

Reviewing files that changed from the base of the PR and between 3ec56ec and 4012d11.

📒 Files selected for processing (1)
  • tests/auto/util/tst_util.cpp

Comment thread tests/auto/util/tst_util.cpp Outdated
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 15, 2026

Coverage Status

Coverage is 21.015%fix/test-improvements into main. No base build found for main.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 27.11%. Comparing base (bfbcdb6) to head (4012d11).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1010   +/-   ##
=======================================
  Coverage   27.11%   27.11%           
=======================================
  Files          39       39           
  Lines        3334     3334           
=======================================
  Hits          904      904           
  Misses       2430     2430           
Flag Coverage Δ
qtpass 27.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nogeenharrie nogeenharrie force-pushed the fix/test-improvements branch from 4012d11 to edf13db Compare April 15, 2026 20:22
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai Bot commented Apr 15, 2026

CodeAnt AI is running Incremental review

@codeant-ai codeant-ai Bot added size:M This PR changes 30-99 lines, ignoring generated files and removed size:M This PR changes 30-99 lines, ignoring generated files labels Apr 15, 2026
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai Bot commented Apr 15, 2026

CodeAnt AI Incremental review completed.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/auto/util/tst_util.cpp`:
- Around line 605-609: The chi-square threshold constant chi2Critical (used in
the QVERIFY2 assertion) is too tight and yields ~5% false failures; increase the
threshold to a more lenient value (e.g., use a higher critical value or compute
it from a safe alpha) so the test is stable in CI, and update the QVERIFY2
message accordingly to reference the new chi2Critical; locate the constant named
chi2Critical and the QVERIFY2 invocation in tst_util.cpp and change the constant
or replace it with a computed critical value before the assertion.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7c9b4bd0-a5f1-4fc9-b92d-2ce816c36dad

📥 Commits

Reviewing files that changed from the base of the PR and between 4012d11 and edf13db.

📒 Files selected for processing (1)
  • tests/auto/util/tst_util.cpp

Comment thread tests/auto/util/tst_util.cpp Outdated
- Add randomness sanity check to generatePassword test
- Add chi-square goodness-of-fit test to boundedRandom test
- Use lenient chi-square threshold (25.0) for CI stability
@nogeenharrie nogeenharrie force-pushed the fix/test-improvements branch from edf13db to 990ae0a Compare April 15, 2026 20:28
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
tests/auto/util/tst_util.cpp (1)

605-610: ⚠️ Potential issue | 🟡 Minor

Chi-square cutoff is still a bit tight for long-term CI stability.

Line 605 uses 25.0; this can still cause intermittent false failures on unbiased RNG output. Consider a more conservative cutoff (e.g., 27.88 for df=9, α≈0.001).

Suggested adjustment
-  const double chi2Critical = 25.0;
+  // df=9; alpha≈0.001 to reduce random CI false failures.
+  const double chi2Critical = 27.88;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/auto/util/tst_util.cpp` around lines 605 - 610, The chi-square cutoff
stored in chi2Critical is too tight (currently 25.0) and can cause intermittent
CI failures; update the value used in the QVERIFY2 assertion to a more
conservative cutoff (e.g., 27.88 for df=9, α≈0.001) so the check that compares
chi2 against chi2Critical in the QVERIFY2 call is less likely to fail; modify
the chi2Critical declaration near the
QVERIFY2(qPrintable(QStringLiteral(...).arg(chi2).arg(chi2Critical))) line
accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@tests/auto/util/tst_util.cpp`:
- Around line 605-610: The chi-square cutoff stored in chi2Critical is too tight
(currently 25.0) and can cause intermittent CI failures; update the value used
in the QVERIFY2 assertion to a more conservative cutoff (e.g., 27.88 for df=9,
α≈0.001) so the check that compares chi2 against chi2Critical in the QVERIFY2
call is less likely to fail; modify the chi2Critical declaration near the
QVERIFY2(qPrintable(QStringLiteral(...).arg(chi2).arg(chi2Critical))) line
accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: a9dfda44-ce21-4229-a6fb-533c881e1e8c

📥 Commits

Reviewing files that changed from the base of the PR and between edf13db and 990ae0a.

📒 Files selected for processing (1)
  • tests/auto/util/tst_util.cpp

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

Labels

size:M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants