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

mining: Improve tests for prio queue. #667

Merged
merged 1 commit into from Apr 14, 2016

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Apr 13, 2016

This improves the tests of the priority queue to include the secondary sort ordering as well as adds some manual entries to ensure the edge conditions are properly tested.

This also brings the priority queue test coverage up to 100%.


This change is Reviewable

@jrick
Copy link
Member

jrick commented Apr 14, 2016

This looks ok. My only suggestion is to use a random seed and if any of the tests fail, then the seed can be printed so the test can be reproduced, but this is still an improvement by itself so that's no reason to hold up the PR.

@davecgh davecgh force-pushed the mining_prioqueue_improve_test branch from 675ff43 to ec82b90 Compare April 14, 2016 04:25
@davecgh
Copy link
Member Author

davecgh commented Apr 14, 2016

Good suggestion. I've updated the PR to create a new random source from a random seed and log the seed.

@davecgh davecgh force-pushed the mining_prioqueue_improve_test branch from ec82b90 to 1527807 Compare April 14, 2016 04:28
@jrick
Copy link
Member

jrick commented Apr 14, 2016

I would only print it if the test failed:

defer func() {
    if t.Failed() {
        t.Logf("Random numbers generated using seed %v", randSeed)
    }
}()

This improves the tests of the priority queue to include the secondary
sort ordering as well as adds some manual entries to ensure the edge
conditions are properly tested.

This also brings the priority queue test coverage up to 100%.
@davecgh davecgh force-pushed the mining_prioqueue_improve_test branch from 1527807 to cbf994e Compare April 14, 2016 05:13
@davecgh
Copy link
Member Author

davecgh commented Apr 14, 2016

Updated to only print seed on failure with defer.

@jrick
Copy link
Member

jrick commented Apr 14, 2016

ok

@davecgh davecgh merged commit e15d300 into btcsuite:master Apr 14, 2016
@davecgh davecgh deleted the mining_prioqueue_improve_test branch April 14, 2016 05:44
cjepson pushed a commit to cjepson/btcd that referenced this pull request Sep 21, 2016
This improves the tests of the priority queue to include the secondary
sort ordering as well as adds some manual entries to ensure the edge
conditions are properly tested.

This also brings the priority queue test coverage up to 100%.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants