-
Notifications
You must be signed in to change notification settings - Fork 38.7k
tests: Remove unused testing code #14312
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
tests: Remove unused testing code #14312
Conversation
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.
Should probably call this:
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 9a589240a8..238b50387d 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -161,8 +161,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
success = TestStatus.FAILED
try:
- if self.options.usecli and not self.supports_cli:
- raise SkipTest("--usecli specified but test does not support using CLI")
+ if self.options.usecli:
+ if not self.supports_cli:
+ raise SkipTest("--usecli specified but test does not support using CLI")
+ self.skip_if_no_cli()
self.skip_test_if_missing_module()
self.setup_chain()
self.setup_network()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.
ack
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.
ack
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.
When has this last been used? Not sure if we want to remove library functions that are still used in other branches. Same for the key.py changes.
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.
Last use was removed in b9f34e8
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.
ack
test/functional/feature_cltv.py
Outdated
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.
When was the first and last use? Might want to remove altogether after investigating that.
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.
Last use was removed in your commit fac3e22 :-)
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.
So please remove the constants here as well.
| No more conflicts as of last run. |
ecb0f6b to
065bedc
Compare
|
@MarcoFalke Thanks for the quick review. Updated. Please re-review :-) |
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.
remove? (This is no longer used after the removal of the comptool)
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.
Done!
065bedc to
9153b77
Compare
9153b77 to
908c888
Compare
|
promag
left a comment
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.
Concept ACK, agree with rationale.
Mind sharing how you find these unused code?
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.
Does this change belong here?
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.
That was suggested by @MarcoFalke in #14312 (comment) :-)
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.
nit, unrelated.
908c888 to
e17da14
Compare
c82190c tests: Add Python dead code linter (vulture) (practicalswift) 590a57f tests: Remove unused testing code (practicalswift) Pull request description: Add Python dead code linter (`vulture`) to Travis. Rationale for allowing dead code only after explicit opt-in (via `--ignore-names`): * Less is more :-) * Unused code is by definition "untested" * Unused code can be an indication of bugs/logical errors. By making the contributor aware of newly introduced unused code it gives him/her an opportunity to investigate if the unused code they introduce is malignant or benign :-) * Unused code is hard to spot for humans and is thus often missed during manual review * [YAGNI](https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it) Based on #14312 to make linter job pass. Tree-SHA512: 4c581df7c34986e226e4ade479e0d3c549daf38f4a4dc4564b25564d63e773a1830ba55d1289c771b1fa325483e8855b82b56e61859fe8e4b7dfa54034b093b6
…Travis c82190c tests: Add Python dead code linter (vulture) (practicalswift) 590a57f tests: Remove unused testing code (practicalswift) Pull request description: Add Python dead code linter (`vulture`) to Travis. Rationale for allowing dead code only after explicit opt-in (via `--ignore-names`): * Less is more :-) * Unused code is by definition "untested" * Unused code can be an indication of bugs/logical errors. By making the contributor aware of newly introduced unused code it gives him/her an opportunity to investigate if the unused code they introduce is malignant or benign :-) * Unused code is hard to spot for humans and is thus often missed during manual review * [YAGNI](https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it) Based on bitcoin#14312 to make linter job pass. Tree-SHA512: 4c581df7c34986e226e4ade479e0d3c549daf38f4a4dc4564b25564d63e773a1830ba55d1289c771b1fa325483e8855b82b56e61859fe8e4b7dfa54034b093b6
…Travis c82190c tests: Add Python dead code linter (vulture) (practicalswift) 590a57f tests: Remove unused testing code (practicalswift) Pull request description: Add Python dead code linter (`vulture`) to Travis. Rationale for allowing dead code only after explicit opt-in (via `--ignore-names`): * Less is more :-) * Unused code is by definition "untested" * Unused code can be an indication of bugs/logical errors. By making the contributor aware of newly introduced unused code it gives him/her an opportunity to investigate if the unused code they introduce is malignant or benign :-) * Unused code is hard to spot for humans and is thus often missed during manual review * [YAGNI](https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it) Based on bitcoin#14312 to make linter job pass. Tree-SHA512: 4c581df7c34986e226e4ade479e0d3c549daf38f4a4dc4564b25564d63e773a1830ba55d1289c771b1fa325483e8855b82b56e61859fe8e4b7dfa54034b093b6
…Travis c82190c tests: Add Python dead code linter (vulture) (practicalswift) 590a57f tests: Remove unused testing code (practicalswift) Pull request description: Add Python dead code linter (`vulture`) to Travis. Rationale for allowing dead code only after explicit opt-in (via `--ignore-names`): * Less is more :-) * Unused code is by definition "untested" * Unused code can be an indication of bugs/logical errors. By making the contributor aware of newly introduced unused code it gives him/her an opportunity to investigate if the unused code they introduce is malignant or benign :-) * Unused code is hard to spot for humans and is thus often missed during manual review * [YAGNI](https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it) Based on bitcoin#14312 to make linter job pass. Tree-SHA512: 4c581df7c34986e226e4ade479e0d3c549daf38f4a4dc4564b25564d63e773a1830ba55d1289c771b1fa325483e8855b82b56e61859fe8e4b7dfa54034b093b6
Remove unused testing code.
Rationale: