Don't share objects between TestInstances #6465

Merged
merged 1 commit into from Jul 24, 2015

Conversation

Projects
None yet
3 participants
@casey
Contributor

casey commented Jul 22, 2015

If two TestInstances are created without providing a list of objects, they'll be shared across both instances.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jul 24, 2015

Member

Good catch. ACK.
(luckily this case doesn't ever trigger as only one test instance is created per test, but still)

Member

laanwj commented Jul 24, 2015

Good catch. ACK.
(luckily this case doesn't ever trigger as only one test instance is created per test, but still)

@laanwj laanwj added the Tests label Jul 24, 2015

@laanwj laanwj merged commit 56b28fc into bitcoin:master Jul 24, 2015

1 check passed

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

laanwj added a commit that referenced this pull request Jul 24, 2015

Merge pull request #6465
56b28fc Don't share objects between TestInstances (Casey Rodarmor)
@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Sep 20, 2015

Member

Nit: This doesn't actually fix the problem if objects is passed as a parameter. It would be better as:

self.blocks_and_transactions = list(objects)

...or maybe a deep-copy.

But maybe callers would want shared objects in some cases?

Without a real use case, it's hard to know what behaviour is sane here.

Member

luke-jr commented Sep 20, 2015

Nit: This doesn't actually fix the problem if objects is passed as a parameter. It would be better as:

self.blocks_and_transactions = list(objects)

...or maybe a deep-copy.

But maybe callers would want shared objects in some cases?

Without a real use case, it's hard to know what behaviour is sane here.

@casey casey deleted the casey:test-instance-objects-sharing-bug branch Sep 20, 2015

@str4d str4d referenced this pull request in zcash/zcash Feb 15, 2017

Merged

Bitcoin 0.12 test PRs 1 #2101

zkbot added a commit to zcash/zcash that referenced this pull request Feb 15, 2017

zkbot added a commit to zcash/zcash that referenced this pull request Feb 20, 2017

zkbot added a commit to zcash/zcash that referenced this pull request Mar 3, 2017

Auto merge of #2101 - str4d:2074-tests, r=arcalinea
Bitcoin 0.12 test PRs 1

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6337
- bitcoin/bitcoin#6390
- bitcoin/bitcoin#5515
- bitcoin/bitcoin#6287 (partial, remainder included in bitcoin/bitcoin#6703)
- bitcoin/bitcoin#6465

Part of #2074.

@dagurval dagurval referenced this pull request in bitcoinxt/bitcoinxt Aug 9, 2017

Merged

Add comptool.RejectResult #238

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment