Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect backslash before multi-lines string #1828

Merged
merged 1 commit into from
Aug 3, 2024

Conversation

LiuYinCarl
Copy link
Contributor

@LiuYinCarl LiuYinCarl commented Jul 30, 2024

The PR is used to fix incorrect backslash before multi-lines string which will result in incorrect lines alignment.

Source code for test

#################### case 1
a = ["aaa",\
     "bbb \
     ccc"]

#################### case 2
b = ("a",\
     "b \
     c")

########## case 3
c = "xxx"\
    "yyy \
    zzz"

#################### case 4
def f(a, b):
    pass

f(100,\
  "hello \
  world")

#################### case 5(good case)
d = """\
hello
"""


def show_incorrect_line_align():
    # line 1
    # line 2
    # line 3
    # line 4
    # line 5
    pass

Run commands

coverage run main.py
coverage html
cd htmlcov
python3 -m http.server 9000

image

@LiuYinCarl LiuYinCarl changed the title fix incorrect backslash before multi-lines string Fix incorrect backslash before multi-lines string Jul 30, 2024
@LiuYinCarl
Copy link
Contributor Author

@nedbat Hi,do you have time to take a kook at this PR? Thanks.

@nedbat
Copy link
Owner

nedbat commented Aug 1, 2024

I will take a look, but it might take me some time.

@nedbat
Copy link
Owner

nedbat commented Aug 3, 2024

BTW, what name should I use for the Changelog and Contributors files?

nedbat added a commit that referenced this pull request Aug 3, 2024
nedbat added a commit that referenced this pull request Aug 3, 2024
nedbat added a commit that referenced this pull request Aug 3, 2024
@nedbat nedbat merged commit 9aaa404 into nedbat:master Aug 3, 2024
35 checks passed
nedbat added a commit that referenced this pull request Aug 3, 2024
nedbat added a commit that referenced this pull request Aug 3, 2024
@nedbat
Copy link
Owner

nedbat commented Aug 3, 2024

Tests added in dc819ff
Changelog updated in da1682f
Thanks!

@nedbat
Copy link
Owner

nedbat commented Aug 4, 2024

This is now released as part of coverage 7.6.1.

github-merge-queue bot pushed a commit to rustymotors/server that referenced this pull request Aug 4, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [coverage](https://github.com/nedbat/coveragepy) | `==7.6.0` ->
`==7.6.1` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/coverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/coverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/coverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/coverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>nedbat/coveragepy (coverage)</summary>

###
[`v7.6.1`](https://github.com/nedbat/coveragepy/blob/HEAD/CHANGES.rst#Version-761--2024-08-04)

[Compare
Source](https://github.com/nedbat/coveragepy/compare/7.6.0...7.6.1)

- Fix: coverage used to fail when measuring code using
:func:`runpy.run_path <python:runpy.run_path>` with a :class:`Path
<python:pathlib.Path>` argument.
    This is now fixed, thanks to `Ask Hjorth Larsen <pull 1819_>`\_.

- Fix: backslashes preceding a multi-line backslashed string could
confuse the
HTML report. This is now fixed, thanks to `LiuYinCarl <pull 1828_>`\_.

- Now we publish wheels for Python 3.13, both regular and free-threaded.

.. \_pull
1819:[nedbat/coveragepy#1819
.. \_pull
1828[nedbat/coveragepy#1828

.. \_changes\_7-6-0:

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/rustymotors/server-old).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
renovate bot added a commit to allenporter/flux-local that referenced this pull request Aug 5, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [coverage](https://github.com/nedbat/coveragepy) | `==7.6.0` ->
`==7.6.1` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/coverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/coverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/coverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/coverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>nedbat/coveragepy (coverage)</summary>

###
[`v7.6.1`](https://github.com/nedbat/coveragepy/blob/HEAD/CHANGES.rst#Version-761--2024-08-04)

[Compare
Source](https://github.com/nedbat/coveragepy/compare/7.6.0...7.6.1)

- Fix: coverage used to fail when measuring code using
:func:`runpy.run_path <python:runpy.run_path>` with a :class:`Path
<python:pathlib.Path>` argument.
    This is now fixed, thanks to `Ask Hjorth Larsen <pull 1819_>`\_.

- Fix: backslashes preceding a multi-line backslashed string could
confuse the
HTML report. This is now fixed, thanks to `LiuYinCarl <pull 1828_>`\_.

- Now we publish wheels for Python 3.13, both regular and free-threaded.

.. \_pull
1819:[nedbat/coveragepy#1819
.. \_pull
1828[nedbat/coveragepy#1828

.. \_changes\_7-6-0:

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/allenporter/flux-local).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
fritterhoff pushed a commit to eduMFA/eduMFA that referenced this pull request Aug 5, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [test/coverage](https://github.com/nedbat/coveragepy) | `==7.6.0` ->
`==7.6.1` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/test%2fcoverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/test%2fcoverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/test%2fcoverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/test%2fcoverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>nedbat/coveragepy (test/coverage)</summary>

###
[`v7.6.1`](https://github.com/nedbat/coveragepy/blob/HEAD/CHANGES.rst#Version-761--2024-08-04)

[Compare
Source](https://github.com/nedbat/coveragepy/compare/7.6.0...7.6.1)

- Fix: coverage used to fail when measuring code using
:func:`runpy.run_path <python:runpy.run_path>` with a :class:`Path
<python:pathlib.Path>` argument.
    This is now fixed, thanks to `Ask Hjorth Larsen <pull 1819_>`\_.

- Fix: backslashes preceding a multi-line backslashed string could
confuse the
HTML report. This is now fixed, thanks to `LiuYinCarl <pull 1828_>`\_.

- Now we publish wheels for Python 3.13, both regular and free-threaded.

.. \_pull
1819:[nedbat/coveragepy#1819
.. \_pull
1828[nedbat/coveragepy#1828

.. \_changes\_7-6-0:

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job log](https://developer.mend.io/github/eduMFA/eduMFA).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@the-13th-letter
Copy link

Tests added in dc819ff

I stumbled across this while reviewing #1838. I think the tests (and this fix) are incomplete.

coveragepy/tests/test_html.py

Lines 1134 to 1154 in dc819ff

def test_bug_1828(self) -> None:
# https://github.com/nedbat/coveragepy/pull/1828
self.make_file("backslashes.py", """\
a = ["aaa",\\
"bbb \\
ccc"]
""")
cov = coverage.Coverage()
backslashes = self.start_import_stop(cov, "backslashes")
cov.html_report(backslashes, directory="out")
contains(
"out/backslashes_py.html",
# line 2 is `"bbb \`
r'<a id="t2" href="#t2">2</a></span>'
+ r'<span class="t"> <span class="str">"bbb \</span>',
# line 3 is `ccc"]`
r'<a id="t3" href="#t3">3</a></span>'
+ r'<span class="t"><span class="str"> ccc"</span><span class="op">]</span>',
)

In lines 1146–1154, you are testing whether the HTML output correctly reproduces the backslash in the "bbb ccc" string. However, the rendered text does not contain the backslash and linebreak within line 1137. This leads to a missing linebreak, and therefore to misaligned coverage information similar to #1836, just in the other direction.

def test_1828(self) -> None:
# https://github.com/nedbat/coveragepy/pull/1828
tokens = list(source_token_lines(textwrap.dedent("""
x = \
1
a = ["aaa",\\
"bbb \\
ccc"]
""")))
assert tokens == [
[],
[('nam', 'x'), ('ws', ' '), ('op', '='), ('ws', ' '), ('num', '1')],
[('nam', 'a'), ('ws', ' '), ('op', '='), ('ws', ' '),
('op', '['), ('str', '"aaa"'), ('op', ','), ('xx', '\\')],
[('ws', ' '), ('str', '"bbb \\')],
[('str', ' ccc"'), ('op', ']')],
]

Testing the same input, just with the tokenizer instead of the HTML report renderer. You can see in line 112 that there is a ws token with many spaces (which doesn't occur in the tokenizer input) instead of two tokens with a backspace continuation (which does occur). Again, this leads to a missing linebreak, and to misaligned coverage info for all following lines.


Concerning this pull request in general:

  • Python 3.11 (Linux) with coverage 7.6.1 (plus commit 01779db from fix backslash in f-string #1838 applied manually): I see the "before fix"/incorrect version for the a variable and the "after fix"/correct version for the b, c, d and f variables. The same happens whether I use single-quoted strings, triple-quoted strings, or triple-quoted f-strings.
  • Python 3.12 (Linux) with coverage 7.6.1 (plus commit 01779db): I see the incorrect version for the a variable if and only if I'm not using triple-quoted f-strings, and the correct version in all other cases/for all other variables.

@the-13th-letter
Copy link

I stumbled across this while reviewing #1838. I think the tests (and this fix) are incomplete.

My apologies, I forgot to ping @nedbat, as the author of these tests.

See also #1838 (comment) for a sample file-plus-coverage-output of a run which is correct according to (my reading of) the aforementioned test functions, despite misaligning coverage info later on in the file. Screenshot:

(screenshot)

@nedbat
Copy link
Owner

nedbat commented Aug 27, 2024

I appreciate all the work you are doing. I'm trying to keep up! You said:

In lines 1146–1154, you are testing whether the HTML output correctly reproduces the backslash in the "bbb ccc" string. However, the rendered text does not contain the backslash and linebreak within line 1137.

When I print the HTML created by this test, I see:

<main id="source">
    <p class="run"><span class="n"><a id="t1" href="#t1">1</a></span><span class="t"><span class="nam">a</span> <span class="op">=</span> <span class="op">[</span><span class="str">"aaa"</span><span class="op">,</span><span class="xx">\</span>&nbsp;</span><span class="r"></span></p>
    <p class="pln"><span class="n"><a id="t2" href="#t2">2</a></span><span class="t">     <span class="str">"bbb \</span>&nbsp;</span><span class="r"></span></p>
    <p class="pln"><span class="n"><a id="t3" href="#t3">3</a></span><span class="t"><span class="str">     ccc"</span><span class="op">]</span>&nbsp;</span><span class="r"></span></p>
</main>

Line 1 ends with backslash, and is separate from line 2. Can you show me more specifically what you mean?

@LiuYinCarl
Copy link
Contributor Author

LiuYinCarl commented Aug 27, 2024

@nedbat maybe it's because the code of @the-13th-letter contain a blank between "," and "\", I fix it in a1aa6b8

@nedbat
Copy link
Owner

nedbat commented Aug 27, 2024

To help with these kinds of tests, I added a new helper to extract the rendered test from an HTML report in 9494ba7.

@the-13th-letter
Copy link

@netbat:

When I print the HTML created by this test, I see:

<main id="source">
    <p class="run"><span class="n"><a id="t1" href="#t1">1</a></span><span class="t"><span class="nam">a</span> <span class="op">=</span> <span class="op">[</span><span class="str">"aaa"</span><span class="op">,</span><span class="xx">\</span>&nbsp;</span><span class="r"></span></p>
    <p class="pln"><span class="n"><a id="t2" href="#t2">2</a></span><span class="t">     <span class="str">"bbb \</span>&nbsp;</span><span class="r"></span></p>
    <p class="pln"><span class="n"><a id="t3" href="#t3">3</a></span><span class="t"><span class="str">     ccc"</span><span class="op">]</span>&nbsp;</span><span class="r"></span></p>
</main>

Line 1 ends with backslash, and is separate from line 2. Can you show me more specifically what you mean?

Currently, the test_bug_1828 function does not inspect or assert anything about the first line of the output; it only checks that the "bbb \<newline>ccc" part of the output is as expected. So both your rendition and my rendition would pass this test, even though my rendition has the "missing linebreak/coverage info misalignment" issue. That's why I'm arguing that the tests are incomplete, because methinks they should be explicitly asserting the comma-(space-)backslash in the first line too.


I'm going to be busy at $WORK for the next couple of days, and will only be able to return to debugging this issue on Saturday (Aug 31st).

@nedbat maybe it's because the code of @the-13th-letter contain a blank between "," and "\", I fix it in a1aa6b8

Sounds plausible. I'll try that out too on Saturday.

@nedbat
Copy link
Owner

nedbat commented Aug 27, 2024

Currently, the test_bug_1828 function does not inspect or assert anything about the first line of the output;

I changed that this morning: now it makes an assertion about all of the rendered text. I hope we can write more tests using the same technique.

@the-13th-letter
Copy link

Currently, the test_bug_1828 function does not inspect or assert anything about the first line of the output;

I changed that this morning: now it makes an assertion about all of the rendered text. I hope we can write more tests using the same technique.

Wonderful. I've taken the liberty of extending test_bug_1828 to cover more cases of whitespace-then-continuation-backslash. On current HEAD (28d22a3), I cannot get this test to break anymore, and it checks all the rendered text(-contents) against our expected output. Please see the attached patch, which passes all linting and all pytest tests (though perhaps not all code style checks) on Python 3.11 and 3.12.

(Installing 3.8, 3.9, 3.10 or PyPy on my system – with development headers – is quite a pain, so I only checked 3.11 and 3.12.)

(Yes, I know, GitHub has proper pull requests, but this would be a pull request on a pull request, and it feels a little overkill for something like this.)
issue_1828_more_whitespace_tests.diff.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants