Skip to content

Conversation

jamesob
Copy link
Contributor

@jamesob jamesob commented Dec 10, 2021

Hopefully fixes #23646.

This makes a few changes to feature_init (along the lines of the discussion here: #23289 (comment)). The changes are detailed in the specific commits, but a summary might be:

  • more robust log-line detection (at the expense of maybe being slower)
  • retain all debug.log content
  • perturb .ldb files in a more complete way

@jamesob jamesob force-pushed the 2021-12-feature-init-test-improvement branch from 2dcd812 to 95aca84 Compare December 10, 2021 19:54
@DrahtBot DrahtBot added the Tests label Dec 10, 2021
@jamesob jamesob force-pushed the 2021-12-feature-init-test-improvement branch from 95aca84 to 2a026e9 Compare December 10, 2021 23:48
@maflcko
Copy link
Member

maflcko commented Dec 13, 2021

Could rebase over commit bf66e25?

@jamesob jamesob force-pushed the 2021-12-feature-init-test-improvement branch from 2a026e9 to 0fc6b72 Compare December 13, 2021 13:21
@jamesob
Copy link
Contributor Author

jamesob commented Dec 13, 2021

Could rebase over commit bf66e25?

Rebased and re-enabled the txindex portion of the test.

@maflcko
Copy link
Member

maflcko commented Dec 15, 2021

Thanks, concept ACK

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 16, 2021

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #10102 (Multiprocess bitcoin by ryanofsky)

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.

@maflcko
Copy link
Member

maflcko commented Dec 29, 2021

Are you still working on this? If not, that is fine, just let us know, so that it can be picked up.

@jamesob
Copy link
Contributor Author

jamesob commented Dec 29, 2021

Oh whoops, didn't see the rebase label.

This part of the test sporadically fails on CI infrastructure. Instead
of perturbing a single .ldb file of each type, move all .ldb files of a
given type to ensure a bad startup.
This test sporadically fails due to the Python test missing log lines
for reasons that are poorly understood. The problem is made worse by the
fact that this test does not retain the log files from iteration to
iteration.

Change the test to do logline detection in a more robust manner (by
using `re.search` on the whole log content) in a way that is comparable
to the existing `assert_debug_log` utility, and retain all debug.log
content from case to case.
@jamesob jamesob force-pushed the 2021-12-feature-init-test-improvement branch from 0fc6b72 to 8904f17 Compare December 29, 2021 18:22
@DrahtBot DrahtBot mentioned this pull request Jan 1, 2022
@maflcko
Copy link
Member

maflcko commented Jan 3, 2022

Merging this to unbreak the CI. I am resetting too many CI build manually every day

@maflcko maflcko merged commit d69af93 into bitcoin:master Jan 3, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 4, 2022
@@ -132,7 +104,8 @@ def check_clean_start():
if line:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

                                       line = logfile.readline()
                                     File "/usr/lib/python3.8/codecs.py", line 322, in decode
                                       (result, consumed) = self._buffer_decode(data, self.errors, final)
                                   UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe2 in position 0: unexpected end of data

https://cirrus-ci.com/task/6633293054476288?logs=ci#L5910

@maflcko maflcko mentioned this pull request Jan 6, 2022
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 25, 2022
Summary:
This part of the test sporadically fails on CI infrastructure. Instead
of perturbing a single .ldb file of each type, move all .ldb files of a
given type to ensure a bad startup.

This is a partial backport of [[bitcoin/bitcoin#23737 | core#23737]]
bitcoin/bitcoin@24fcf6e

Depends on D12612

Test Plan:

`ninja check-functional`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D12613
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 25, 2022
Summary:
> test: introduce TestNode.debug_log_bytes

> test: add TestNode.wait_for_debug_log

> test: feature_init: retain debug.log and improve detection
>
> This test sporadically fails due to the Python test missing log lines
> for reasons that are poorly understood. The problem is made worse by the
> fact that this test does not retain the log files from iteration to
> iteration.
>
> Change the test to do logline detection in a more robust manner (by
> using `re.search` on the whole log content) in a way that is comparable
> to the existing `assert_debug_log` utility, and retain all debug.log
> content from case to case.

This is a partial backport of [[bitcoin/bitcoin#23737 | core#23737]]
bitcoin/bitcoin@a2fb62b
bitcoin/bitcoin@a8ffbc0
bitcoin/bitcoin@93db6d8

Depends on D12613

Test Plan: `ninja check-functional`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D12614
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 25, 2022
Summary:
> Now that [[bitcoin/bitcoin#23365 | core#23365]] is merged.

[[bitcoin/bitcoin#23365 | core#23365]] was backported in D11299

This concludes backport of [[bitcoin/bitcoin#23737 | core#23737]]
bitcoin/bitcoin@8904f17

Depends on D12614

Test Plan: `ninja check-functional`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D12615
@bitcoin bitcoin locked and limited conversation to collaborators Jan 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: timeouts in feature_init.py
3 participants