test: add test for specifying custom pidfile via `-pid` #30724

This small PR adds test coverage for the -pid command line option, which allows to overrule the pid filename (bitcoind.pid by default). One can specify either a relative path (within the datadir) or an absolute one; the latter is tested using self.options.tmpdir. Note that the functional test file feature_init.py so far only contained a stress test; with this new sub-test added, both the description and the test name are adapted to be more generic.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage

For detailed information about the code coverage, see the test coverage report.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Concept ACK Good addition. CR and ran tests locally.

"bitcoind.pid" is also mentioned in feature_filelock.py. Could add a commit that deduplicates this.

Created an example commit here: https://github.com/tdb3/bitcoin/commit/b622390b33cf3813b4f80d13377b248d6e898a4f

Thought about maybe making something like a

[@property](/bitcoin-bitcoin/contributor/property/)
def pid_file_path(self) -> Path:
    return self.chain_path / "bitcoind.pid"

but with bitcoind -pid=someotherfile being an option, might be confusing/unexpected to have pid_file_path mismatch the pid file actually used by the node.

Concept ACK +1 on the suggestion above to move BITCOIN_PID_FILENAME_DEFAULT and import it in both files.

This is out of scope for this PR, so feel free to ignore. it would be good to add an edge case to test starting a node when there's already a pre-existing PID file.

@tdb3:

"bitcoind.pid" is also mentioned in feature_filelock.py. Could add a commit that deduplicates this.

Created an example commit here: tdb3@b622390

Thanks! I've cherry-picked your commit, but reordered it to appear before the test introduction commit and changed the commit subject accordingly (introduce constant, instead of deduplicating). Let me know if that's okay for you. @naiyoma:

This is out of scope for this PR, so feel free to ignore. it would be good to add an edge case to test starting a node when there's already a pre-existing PID file.

Good idea. I decided to leave that for a follow-up, happy to review if someone opens a PR.

Thanks! I've cherry-picked your commit, but reordered it to appear before the test introduction commit and changed the commit subject accordingly (introduce constant, instead of deduplicating). Let me know if that's okay for you.

Looks great, thanks.

ACK 04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a

Code review ACK 04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a

Tested ACK https://github.com/bitcoin/bitcoin/pull/30724/commits/04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a

ACK 04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a