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

add rpc_misc.py, mv test getmemoryinfo, add test mallocinfo #15485

Merged
merged 2 commits into from Mar 1, 2019

Conversation

Projects
None yet
6 participants
@adamjonas
Copy link
Contributor

commented Feb 26, 2019

Creating the rpc_misc.py functional test file to add space for adding tests to a file that doesn't have a lot of coverage.

  • Removing the getmemoryinfo() smoke test from wallet basic rather than moving it to keep the wallet decoupled. Feel like testing for reasonable memory allocation values should suffice.
  • Adding coverage for mallocinfo(). Introduced standard lib XML parser since the function exports an XML string that describes the current state of the memory-allocation implementation in the caller.
@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

utACK 2e957b6

Good to split up the overly large wallet_basic tests into independent tests, that are easier to debug in case of failure.

@MarcoFalke
Copy link
Member

left a comment

Oops, one assert issue I missed

Show resolved Hide resolved test/functional/rpc_misc.py Outdated
Show resolved Hide resolved test/functional/rpc_misc.py Outdated

@adamjonas adamjonas force-pushed the adamjonas:test_rpc_misc branch from 2e957b6 to 8032a97 Feb 26, 2019

@jnewbery
Copy link
Member

left a comment

Looks great. Thanks for doing this.

You'll need to update the file mode bits so this is executable. Run chmod 755 rpc_misc.py, then commit the file and push.

I've left some style nits inline. Up to you whether you take them or not.

Show resolved Hide resolved test/functional/rpc_misc.py Outdated
Show resolved Hide resolved test/functional/rpc_misc.py Outdated
Show resolved Hide resolved test/functional/rpc_misc.py Outdated
Show resolved Hide resolved test/functional/rpc_misc.py Outdated

@adamjonas adamjonas force-pushed the adamjonas:test_rpc_misc branch from 8032a97 to 2fa85eb Feb 26, 2019

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

re-utACK 2fa85eb

@jnewbery

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

ACK 2fa85eb. Nice work!

@practicalswift

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

Concept ACK

What an excellent first-time contribution! Hope I'll see more contributions from you going forward! :-)

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Coverage

Coverage Change (pull 15485, 26ffd3c) Reference (master, d88f7f8)
Lines +0.0231 % 87.3445 %
Functions +0.0147 % 84.7625 %
Branches +0.0069 % 51.4908 %

Updated at: 2019-02-26T21:19:47.481840.

@fanquake fanquake added the Tests label Feb 26, 2019

assert_greater_than(memory['used'], 0)
assert_greater_than(memory['free'], 0)
assert_greater_than(memory['total'], 0)
assert_greater_than(memory['locked'], 0)

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Feb 28, 2019

Member

This fails on appveyor:

                                   AssertionError: 0 <= 0

This comment has been minimized.

Copy link
@jnewbery

jnewbery Feb 28, 2019

Member

See assert_greater_than_or_equal()

# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test RPC misc output."""
import xml.etree.ElementTree as ET

This comment has been minimized.

Copy link
@practicalswift

practicalswift Feb 28, 2019

Member

Worth being aware of regarding the security of xml.etree.ElementTree:

Warning The xml.etree.ElementTree module is not secure against maliciously constructed data. If you need to parse untrusted or unauthenticated data see XML vulnerabilities.

We'll only be parsing trusted data from what I can think of, so likely not a problem in the testing code :-)

This comment has been minimized.

Copy link
@adamjonas

adamjonas Mar 1, 2019

Author Contributor

Thanks for flagging @practicalswift. I was looking for other examples of xml libraries and this precedent that I tried to follow.

@adamjonas adamjonas force-pushed the adamjonas:test_rpc_misc branch from 5e17ac9 to f13ad1c Mar 1, 2019

@MarcoFalke MarcoFalke merged commit f13ad1c into bitcoin:master Mar 1, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Mar 1, 2019

Merge #15485: add rpc_misc.py, mv test getmemoryinfo, add test malloc…
…info

f13ad1c modify test for memory locked in case locking pages failed at some point (Adam Jonas)
2fa85eb add rpc_misc.py, mv test getmemoryinfo, add test mallocinfo (Adam Jonas)

Pull request description:

  Creating the `rpc_misc.py` functional test file to add space for adding tests to a file that doesn't have a lot of coverage.
    - Removing the `getmemoryinfo()` smoke test from wallet basic rather than moving it to keep the wallet decoupled. Feel like testing for reasonable memory allocation values should suffice.
    - Adding coverage for `mallocinfo()`. Introduced standard lib XML parser since the function exports an XML string that describes the current state of the memory-allocation implementation in the caller.

Tree-SHA512: ced30115622916c88d1e729969ee331272ec9f2881eb36dee4bb7331bf633a6810a57fed63a0cfaf86de698edb5162e6a035efd07c89ece1df56b69d61288072
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.