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

Fix TestDNS_ServiceLookup_ARecordLimits so that it only creates test agents the minimal amount of time #21608

Merged
merged 6 commits into from
Aug 15, 2024

Conversation

jmurret
Copy link
Member

@jmurret jmurret commented Aug 15, 2024

Description

TestDNS_ServiceLookup_ARecordLimits is one of our longest running tests.

It's average runtime in CI over the last 3 months is 1min 45 secs.

Locally on a mac M1, it performs in 1 min 11 seconds:
Screenshot 2024-08-15 at 7 04 21 AM

The reason why is that for every test case it spawns a new TestAgent. This is 112 test agents getting spawned, taking up ports, file descriptors, etc.

Re-arranging this test so that test agents are only created when the agent config changes, reduces it to 7 Test Agents getting created. (There was also a category of tests that could be removed as duplicates that removed 28 of the test agent).

This is the test results locally now that finish in 10 secs:
Screenshot 2024-08-15 at 9 20 19 AM

Testing & Reproduction steps

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@jmurret jmurret added pr/no-changelog PR does not need a corresponding .changelog entry backport/1.19 This release series is longer active on CE, use backport/ent/1.19 labels Aug 15, 2024
Copy link
Member

@zalimeni zalimeni 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 very much for finding and trimming these 🙏🏻

…gents the minimal amount of time. (#21609)

Fix TestDNS_ServiceLookup_AnswerLimits  so that it only creates test agents the minimal amount of time
@jmurret jmurret enabled auto-merge (squash) August 15, 2024 17:48
@jmurret jmurret merged commit f76da16 into main Aug 15, 2024
89 checks passed
@jmurret jmurret deleted the jm/a-record-limits branch August 15, 2024 18:09
philrenaud pushed a commit that referenced this pull request Sep 12, 2024
…agents the minimal amount of time (#21608)

* get rid of unused column

* get rid of duplicate section now that deletion of unused column makes the section duplicate..

* explicit set protocol rathern than infer it in checkDNSService

* explicit have attribute for whether to set EDNS0 in the test cases  rathern than infer it in checkDNSService

* now modify so that test agents are only created for each unique configuration which is based on the a_record_limit.

* Fix TestDNS_ServiceLookup_AnswerLimits  so that it only creates test agents the minimal amount of time. (#21609)

Fix TestDNS_ServiceLookup_AnswerLimits  so that it only creates test agents the minimal amount of time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.19 This release series is longer active on CE, use backport/ent/1.19 pr/no-changelog PR does not need a corresponding .changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants