Skip to content

test: Ignore UTF-8 errors in assert_debug_log#28035

Merged
fanquake merged 3 commits into
bitcoin:masterfrom
maflcko:2307-test-utf8-
Jul 26, 2023
Merged

test: Ignore UTF-8 errors in assert_debug_log#28035
fanquake merged 3 commits into
bitcoin:masterfrom
maflcko:2307-test-utf8-

Conversation

@maflcko
Copy link
Copy Markdown
Member

@maflcko maflcko commented Jul 6, 2023

Fix two bugs, see commit messages.

@DrahtBot
Copy link
Copy Markdown
Contributor

DrahtBot commented Jul 6, 2023

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK theStack

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

@maflcko maflcko force-pushed the 2307-test-utf8- branch 2 times, most recently from fa2ff23 to fa842a3 Compare July 6, 2023 13:36
@DrahtBot DrahtBot removed the CI failed label Jul 6, 2023
@fanquake fanquake requested a review from theStack July 7, 2023 13:20
Copy link
Copy Markdown

@www222fff www222fff left a comment

Choose a reason for hiding this comment

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

Is this fix applied to both for python2 and python3?

@fanquake
Copy link
Copy Markdown
Member

Is this fix applied to both for python2 and python3?

We only support Python 3.

@maflcko maflcko added this to the 26.0 milestone Jul 14, 2023
MarcoFalke added 3 commits July 20, 2023 09:15
debug_log_bytes returned "an opaque number when in text mode"
(https://docs.python.org/3/tutorial/inputoutput.html#methods-of-file-objects),
not the number of bytes.

Fix this by using binary mode or text mode (with the same encoding)
consistently when opening the file for ftell() and read().
read() fails in text mode when the unicode hasn't been fully written
yet. Fixes: "wallet_importdescriptors.py: can't decode bytes in position
228861-228863: unexpected end of data"
(bitcoin#28030)
This fixes a bug in the linter:

"""
Python's open(...) seems to be used to open text files without explicitly specifying encoding='utf8':
test/functional/test_framework/test_node.py:        with open(self.debug_log_path, **kwargs) as dl:
"""
@maflcko
Copy link
Copy Markdown
Member Author

maflcko commented Jul 20, 2023

Split into more commits

Copy link
Copy Markdown
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

utACK fa3d729

Changes look correct to me and should fix #28030.
(Background information for other reviewers: wait_for_debug_log was changed to open the debug file in binary mode in PR #25294)

@fanquake fanquake merged commit 54fe963 into bitcoin:master Jul 26, 2023
@maflcko maflcko deleted the 2307-test-utf8- branch July 26, 2023 08:37
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 9, 2023
fa3d729 lint: Ignore check_fileopens failure on **kwargs (MarcoFalke)
fa6bb85 test: Ignore UTF-8 errors in assert_debug_log (MarcoFalke)
fa63326 test: Fix debug_log_size helper (MarcoFalke)

Pull request description:

  Fix two bugs, see commit messages.

ACKs for top commit:
  theStack:
    utACK fa3d729

Tree-SHA512: 4a29bdf954bf62bb7676c2a41b03ad017bc86d535b2bd912c96bd41d1621beb06d840b53c211480ad51974e8b293bbae620060d2528d269159f32c0b44e47712
kwvg added a commit to kwvg/dash that referenced this pull request Aug 24, 2023
@bitcoin bitcoin locked and limited conversation to collaborators Jul 25, 2024
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.

5 participants