Skip to content

[3.11] gh-94816: Improve coverage of decode_linetable (GH-94853) - #94884

Closed
miss-islington wants to merge 1 commit into
python:3.11from
miss-islington:backport-20b9d2a-3.11
Closed

miss-islington wants to merge 1 commit into
python:3.11from
miss-islington:backport-20b9d2a-3.11

Conversation

@miss-islington

@miss-islington miss-islington commented Jul 15, 2022 •

Copy link
Copy Markdown
Contributor

This makes calls to co_lnotab to exercise this code, as well
as generating synthetically large code to exercise the corner
cases where line numbers need multiple bytes.

Automerge-Triggered-By: GH:brandtbucher
(cherry picked from commit 20b9d2a)

Co-authored-by: Michael Droettboom mdboom@gmail.com

Automerge-Triggered-By: GH:brandtbucher

This makes calls to co_lnotab to exercise this code, as well
as generating synthetically large code to exercise the corner
cases where line numbers need multiple bytes.

Automerge-Triggered-By: GH:brandtbucher
(cherry picked from commit 20b9d2a)

Co-authored-by: Michael Droettboom <mdboom@gmail.com>
@miss-islington

Copy link
Copy Markdown
Contributor Author

Status check is done, and it's a success ✅ .

@brandtbucher

Copy link
Copy Markdown
Member

Hm, this backport revealed a weakness in the test: every time we change the number of instructions (including caches) in any of these code objects, these tests will need to be updated. I don't even think that just checking the length is future-proof?

@miss-islington

Copy link
Copy Markdown
Contributor Author

@mdboom: Status check is done, and it's a success ❌ .

@mdboom

mdboom commented Jul 18, 2022 •

Copy link
Copy Markdown
Contributor

Yikes. Yes, perhaps we revert this. My main concern for coverage here was that when there are 4-byte line numbers that things still make sense / don't crash. I think as long as we can assert that somehow, we can remove the doctest part of this entirely.

EDIT: I confused this with another coverage issue. In this case, we just don't call decode_linetable at all.

@iritkatriel

Copy link
Copy Markdown
Member

@markshannon has created #90916, and I guess the same applies to test_code as well.

We should have a test for each of the features, but not a large number of test cases that all test many things.

@mdboom

mdboom commented Jul 18, 2022

Copy link
Copy Markdown
Contributor

We should close this (I don't have permissions) and I filed a PR to revert this on main: #94982

@miss-islington
miss-islington deleted the backport-20b9d2a-3.11 branch January 2, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants