shutdown: Destroy kernel last, make test shutdown order consistent

[TheCharlatan]

The destruction/resetting of node context members in the tests should roughly follow the behavior of the Shutdown function in init.cpp.

This was originally requested by MarcoFalke in this comment in response to the original pull request introducing the kernel::Context.

[DrahtBot]

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

Code Coverage

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ryanofsky, maflcko, achow101

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

[ryanofsky]

Code review ACK 960112d

[maflcko]

Are you still working on this?

[TheCharlatan]

Yes.

[maflcko]

I think there is an outstanding comment: #28224 (comment) ?

[TheCharlatan]

Updated 960112d -> 99a668c (testSetupDestructorOrder_0 -> testSetupDestructorOrder_1, compare)

• Addressed @ryanofsky's comment, ammending commit to adjust the reset order in the Shutdown function.

[ryanofsky]

Code review ACK 99a668c

Changes look good, but I would suggest changing the PR title to something like: "shutdown: destroy kernel last and make test shutdown order consistent" because the current title "tests: Reset node context members on ~ChainTestingSetup" makes it seems like this is a test-only change.

Also think it would be better to split changes in init.cpp and setup_common.cpp into separate commits. The changes are both related to shutdown but they have different rationales and effects, so it doesn't really make sense for them to be in the same commit.

[TheCharlatan]

Updated 99a668c -> c1144f0 (testSetupDestructorOrder_1 -> testSetupDestructorOrder_2, compare)

• No code change, just addressed @ryanofsky's comment, splitting up the commit.

[ryanofsky]

Code review ACK c1144f0. No code changes since last review, just updated commits and descriptions

[maflcko]

ACK c1144f0 🗣

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK c1144f0076339c775f41d4b5fcfdc72191440d96 🗣
T9dJ5U2qRZJzAofSxNNeOCDY1x9g1v2UzSnLfzGQ+Ge86LGQmtA47PGOhyV4baDkH4joyvRrBHb2K0taFvI/Dw==

[achow101]

ACK c1144f0