test: include two more interruptions points #23782

This PR aims to introduce 2 more interruption points in the process of initialization, in order to make thefeature_inittestcase more complete. These are the following:

- Checking all blk files are present - init message: Starting network threads

It is a small improvement for increasing the coverage of potential interruptions, and making sure that the node can restart successfully after these interruptions.

Concept ACK

Increasing number of interruption points in the test will make the test more rigorous as the logfile will be scrutinized with more sentences and phrases.

I have some suggestions to improve upon this PR.

Firstly, how about also adding the following phrases to the lines_to_interuppt_after list

• ‘Validating signatures for all blocks’
• ‘Starting HTTP server.’

Secondly, you should reorder the lines in the lines_to_interuppt_after list according to the order in which they appear in the logfile. Using the following patch, you can quickly know the line number in which the phrase appears.

if line and terminate_line.lower() in line.lower():
-                    self.log.debug(f"Terminating node after {num_lines} log lines seen")
+                    self.log.info(f"Terminating node after {num_lines} log lines seen")
                     sigterm_node()
                     break

When the test is run, this patch will print the phrase with the line number on the terminal.

@shaavan your feedback makes a lot of sense. Thank you. I've included your suggested phrases and also re-ordered the array, according to the order they appear on the logs (I didn't notice that they were following this order). I've changed the command self.log.debug to self.log.info for checking the lines. Do you suggest me to also include this change on the PR? or should I leave it back to self.log.debug?

ACK https://github.com/bitcoin/bitcoin/pull/23782/commits/618f4d289053ad7b8802c709f1d55e516b465573, pending CI

More init coverage is better. :+1:

[Update] Seems that ci is failing for some reason. All the steps of interrupting and re-starting the node seem that are working fine though. Working on debugging. https://cirrus-ci.com/task/4602015652249600?logs=ci#L4088

@seaona probably related to #23737.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #23737 (test: make feature_init more robust by jamesob)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

ACK 618f4d289053ad7b8802c709f1d55e516b465573

This is a nice improvement, and welcome as a first-time contributor 🥃. I ran the test locally, and the CI failure looks unrelated.

The test looks a lot more complete and in order now!

Do you suggest me to also include this change on the PR? or should I leave it back to self.log.debug?

This was just a temporary patch to know the line number of each phrase occurrence. It is better the way it is in the master branch. You wouldn't like to crowd up your terminal each time you run the test :)

I rerun the test with the patch, and I observed there is still some phrases not following the ordering:

Edit: Since this PR is now merged, maybe correcting the ordering could be taken up in a follow-up PR that touches this section.

This was just a temporary patch to know the line number of each phrase occurrence. It is better the way it is in the master branch. You wouldn't like to crowd up your terminal each time you run the test :) @shaavan, I also thought that logging too much info would be over-crowding the terminal, so I left it as it was previously. The command was very useful for debugging though :)

Edit: Since this PR is now merged, maybe correcting the ordering could be taken up in a follow-up PR that touches this section.

I didn't realize 2 lines where the other way around, thank you very much for pointing this and for all your help!