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.

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

Jump to bottom

DnsRecordResolverTest: Use seeded random number generator for deterministic test result #1306

Merged
merged 4 commits into from
Feb 12, 2025
Merged

DnsRecordResolverTest: Use seeded random number generator for deterministic test result #1306

merged 4 commits into from
Feb 12, 2025

Conversation

sunkup
Copy link
Member

@sunkup sunkup commented Feb 11, 2025 •
edited
Loading

Purpose

The test testBestSRVRecord_MultipleRecords_Priority_Same fails in rare occasions.

Short description

Use a seeded random number generator for 100% deterministic results.

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

@sunkup sunkup added the bug Something isn't working label Feb 11, 2025
@sunkup sunkup self-assigned this Feb 11, 2025
@sunkup sunkup linked an issue Feb 11, 2025 that may be closed by this pull request
DnsRecordResolverTest: testBestSRVRecord_MultipleRecords_Priority_Same not 100% deterministic (but should be) #1300
Closed
@sunkup sunkup requested review from rfc2822 and ArnyminerZ February 11, 2025 11:33
@sunkup sunkup marked this pull request as ready for review February 11, 2025 11:46
ArnyminerZ
ArnyminerZ previously approved these changes Feb 11, 2025
View reviewed changes
Copy link
Member

@ArnyminerZ ArnyminerZ left a comment

Choose a reason for hiding this comment

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

Sounds logical to me

@sunkup sunkup requested a review from rfc2822 February 12, 2025 14:09
rfc2822
rfc2822 approved these changes Feb 12, 2025
View reviewed changes
Copy link
Member

@rfc2822 rfc2822 left a comment

Choose a reason for hiding this comment

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

Nice :)

@rfc2822 rfc2822 changed the title Use seeded random number generator for a 100% deterministic test result [DnsRecordResolverTest] Use seeded random number generator for deterministic test result Feb 12, 2025
@rfc2822 rfc2822 changed the title [DnsRecordResolverTest] Use seeded random number generator for deterministic test result DnsRecordResolverTest: Use seeded random number generator for deterministic test result Feb 12, 2025
@rfc2822
Copy link
Member

rfc2822 commented Feb 12, 2025

I have added DnsRecordResolverTest to the title, because the PR title will appear as commit message / entry in the changelogs and without DnsRecordResolverTest it doesn't make much sense

@rfc2822 rfc2822 merged commit d3c1dbb into main-ose Feb 12, 2025
10 checks passed
@rfc2822 rfc2822 deleted the 1300-dnsrecordresolvertest-testbestsrvrecord_multiplerecords_priority_same-not-100-deterministic-but-should-be branch February 12, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfc2822 rfc2822 rfc2822 approved these changes

@ArnyminerZ ArnyminerZ ArnyminerZ left review comments

Assignees

@sunkup sunkup

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DnsRecordResolverTest: testBestSRVRecord_MultipleRecords_Priority_Same not 100% deterministic (but should be)
3 participants
@sunkup @rfc2822 @ArnyminerZ