Page MenuHomePhabricator

tests: Remove unused testing code
ClosedPublic

Authored by PiRK on Oct 19 2020, 15:49.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABC6ef2cd8aeae1: tests: Remove unused testing code
Summary

Removal of dead python code

Partial backport of PR14365
Commit https://github.com/bitcoin/bitcoin/pull/14365/commits/590a57fdecc2eb2e8208d82e491d09a1986e4f0e
Depends on D7981 (backported out of order)

Test Plan

ninja && ninja check-functional

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Owners added a reviewer: Restricted Owners Package.Oct 19 2020, 15:49
PiRK requested review of this revision.Oct 19 2020, 15:49

Where is part 2?

I will work on adding the corresponding linter tomorrow, and take some to time to properly check that it does not degrade performances too much. Fabien told me that external linters are slower than a regex in PHP.

I already ran the command locally. There will be quite a few unused functions, variables and classes to be removed. I imagine I will do that in a separate diff too.

In D7987#187502, @PiRK wrote:

Where is part 2?

I will work on adding the corresponding linter tomorrow, and take some to time to properly check that it does not degrade performances too much. Fabien told me that external linters are slower than a regex in PHP.

It turns out that this tool is not very reliable, for our codebase. vulture --min-confidence=60 finds many false positives, and vulture --min-confidence=70 does not detect obviously unused code added on purpose for testing.
It was disabled in Core: https://github.com/bitcoin/bitcoin/pull/16961

I think the problem is related to the way test_runner.py dynamically finds tests by doing an os.listdir and then runs them as external processes, instead of doing python imports and running functions. We basically use python as a scripting language.

This revision is now accepted and ready to land.Oct 20 2020, 14:24