-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Speed up testing #4239
Speed up testing #4239
Conversation
Another 10s can be shaved off as we fix #4238, meaning all tests run 10s which looks like nice ballpark figure to make local dev. fun again |
let me fix the timing as I took the wrong value from |
* make notification run in the background, this recudes the test_readme time from 18s to 0.10s * reduce time for zone reload * TestServeDNSConcurrent remove entirely. This took a whopping 58s for ... ? A few minutes staring didn't reveal wth it is actually testing. Making values smaller revealed race conditions in the tests. Remove entirely. * Move many interval values to variables so we can reset them to short values for the tests. * test_large_axfr: make the zone smaller. The number used 64K has no rational, make it 64/10 to speed up. * TestProxyThreeWay: use client with shorter timeout A few random tidbits in other tests. Total time saved: 177s (almost 3m) - which makes it worthwhile again to run the test locally: this branch: ~~~ ok github.com/coredns/coredns/test 10.437s cd plugin; time go t ./... 5,51s user 7,51s system 11,15s elapsed 744%CPU ( ~~~ master: ~~~ ok github.com/coredns/coredns/test 35.252s cd plugin; time go t ./... 157,64s user 15,39s system 50,05s elapsed 345%CPU () ~~~ tests/ -25s plugins/ -40s This brings the total on 20s, and another 10s can be saved by fixing dnstapio. Moving this to 5s would be even better, but 10s is also nice. Signed-off-by: Miek Gieben <miek@miek.nl>
ah flakyness in travis (of course it's travis) |
travis is becoming very very slow |
I'm a fan of CircleCI 😁 |
yes, would also cut down on external deps, circle-ci is already finished, and that does a minikube k8s test! |
hmm, my initial commit msg had section headers (#) but these where filtered out because that text is semi-markdown and #-sign are comments... damnit |
Codecov Report
@@ Coverage Diff @@
## master #4239 +/- ##
==========================================
- Coverage 56.06% 55.83% -0.23%
==========================================
Files 223 223
Lines 9850 9852 +2
==========================================
- Hits 5522 5501 -21
- Misses 3866 3888 +22
- Partials 462 463 +1
Continue to review full report at Codecov.
|
if there are no objects I will merge this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
make notification run in the background, this recudes the test_readme
time from 18s to 0.10s
reduce time for zone reload
TestServeDNSConcurrent remove entirely. This took a whopping 58s for
... ? A few minutes staring didn't reveal wth it is actually testing.
Making values smaller revealed race conditions in the tests. Remove
entirely.
Move many interval values to variables so we can reset them to short
values for the tests.
test_large_axfr: make the zone smaller. The number used 64K has no
rational, make it 64/10 to speed up.
TestProxyThreeWay: use client with shorter timeout
A few random tidbits in other tests.
Total time saved: 177s (almost 3m) - which makes it worthwhile again to
run the test locally:
this branch:
master:
tests/ -25s
plugins/ -152s
Signed-off-by: Miek Gieben miek@miek.nl
1. Why is this pull request needed and what does it do?
2. Which issues (if any) are related?
3. Which documentation changes (if any) need to be made?
4. Does this introduce a backward incompatible change or deprecation?