tests: Add unit testing for the CompressScript function #17220

pull adamjonas wants to merge 1 commits into bitcoin:master from adamjonas:add_compress_test_cases changing 1 files +73 −0

adamjonas commented at 8:01 PM on October 22, 2019: member
Salvaging #15104 which adds unit tests for CompressScript function in compressor.cpp

Tested following cases for the CScript:
- CKeyID
- CScriptID
- Uncompressed CPubKey (of size: 65)
- Compressed CPubKey (of size: 32)
fanquake added the label Tests on Oct 22, 2019
in src/test/compress_tests.cpp:74 in 3208906574 outdated
```
  69 | +    key.MakeNewKey(true);
  70 | +    CPubKey pubkey = key.GetPubKey();
  71 | +
  72 | +    CScript script;
  73 | +    script.clear();
  74 | +    script << OP_DUP << OP_HASH160 << ToByteVector(pubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
```
theStack commented at 12:24 AM on October 23, 2019:

nit: This could be put in one line by immediately assigning the script on the declaration: CScript script = CScript() << OP_DUP ..... << OP_CHECKSIG (also applies to the other three functions).

adamjonas commented at 3:14 PM on October 23, 2019:

wouldn't assigning the script on the declaration remove the opportunity to call .clear() on it?

theStack commented at 3:35 PM on October 23, 2019:

I don't think that calling .clear() is needed here at all, since newly constructed CScripts are empty anyway?

dongcarl commented at 3:55 PM on October 23, 2019:

I don't think we need to clear() it, since the default constructor CScript() constructs a (logically) empty CScript.
laanwj added the label Validation on Oct 23, 2019
laanwj removed the label Validation on Oct 23, 2019
laanwj commented at 8:39 AM on October 23, 2019: member

ACK, thanks for adding tests.
dongcarl approved
dongcarl commented at 3:57 PM on October 23, 2019: member

ACK 3208906574ed07cf9c8ae7a6efa69dc20890f664 Read code, ran tests, feel free to ignore the nit!
adamjonas force-pushed on Oct 23, 2019
adamjonas force-pushed on Oct 23, 2019
Add unit testing for the CompressScript functions b05ec410f2
adamjonas force-pushed on Oct 23, 2019
theStack commented at 9:18 PM on October 23, 2019: member

ACK https://github.com/bitcoin/bitcoin/commit/b05ec410f2d9f209796a5df31860e23efd729dfe
laanwj referenced this in commit 37855ec9df on Oct 25, 2019
laanwj merged this on Oct 25, 2019
laanwj closed this on Oct 25, 2019
adamjonas deleted the branch on Oct 25, 2019
jasonbcox referenced this in commit c6b104465b on Oct 30, 2020
PastaPastaPasta referenced this in commit c7abbd5096 on Nov 1, 2021
PastaPastaPasta referenced this in commit 38a0fbee54 on Nov 3, 2021
pravblockc referenced this in commit 13228b77da on Nov 18, 2021
MarcoFalke locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#15104 Tests: Add unit testing for the CompressScript function