test_hacking fails for python 3.6.7 and newer

Bug #1804062 reported by Chris Dent on 2018-11-19
22
This bug affects 4 people
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
High
Stephen Finucane
Ubuntu Cloud Archive
High
Unassigned
Queens
High
Unassigned
Rocky
High
Unassigned
Stein
High
Unassigned
masakari
Undecided
Unassigned
nova (Ubuntu)
High
Unassigned
Bionic
High
Unassigned
Cosmic
High
Unassigned
Disco
High
Unassigned
Eoan
High
Unassigned

Bug Description

The check for double words in test_hacking is failing in python 3.6.7 (released in ubuntu 18.04 within the last few days) and in new versions of 3.7.x. This is is because of this change to python: https://bugs.python.org/issue33899 .

This is causing failures in python 36 unit tests for nova.

The fix ought to be adding a newline to the code sample. Maybe.

Chris Dent (cdent) wrote :

> The fix ought to be adding a newline to the code sample. Maybe.

Nope. Still digging.

Matt Riedemann (mriedem) on 2018-11-19
Changed in nova:
status: New → Confirmed
Matt Riedemann (mriedem) on 2018-11-19
Changed in nova:
importance: Undecided → High

Reviewed: https://review.openstack.org/618843
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=9875c37d9a09ee7160f13c9771cf14ce02cec027
Submitter: Zuul
Branch: master

commit 9875c37d9a09ee7160f13c9771cf14ce02cec027
Author: Chris Dent <email address hidden>
Date: Mon Nov 19 21:41:08 2018 +0000

    Skip double word hacking test

    Because of a change [1] in the tokenize package in the stdlib of
    recent pythons, one of the tests for the hacking checks can fail.
    This change skips the test on newer pythons and leaves a TODO
    to fix it.

    [1] see https://bugs.python.org/issue33899 and
        https://bugs.python.org/issue35107

    Change-Id: I64744a8144fcf630eea609eb2b2d14974f4fd4bb
    Related-Bug: #1804062

Corey Bryant (corey.bryant) wrote :

I'm also hitting this on Bionic (bionic-proposed) with the final 2.7.15 release. So this is currently seen on bionic and above with 2.7/3.6/3.7. Ubuntu package versions can be tracked at:

https://launchpad.net/ubuntu/+source/python2.7
https://launchpad.net/ubuntu/+source/python3.6
https://launchpad.net/ubuntu/+source/python3.7

Changed in cloud-archive:
status: New → Triaged
Changed in nova (Ubuntu):
status: New → Triaged
Changed in cloud-archive:
importance: Undecided → High
Changed in nova (Ubuntu):
importance: Undecided → High
Changed in nova (Ubuntu Bionic):
importance: Undecided → High
status: New → Triaged
Changed in nova (Ubuntu Cosmic):
importance: Undecided → High
status: New → Triaged
Changed in nova (Ubuntu Disco):
importance: Undecided → High
status: New → Triaged
Corey Bryant (corey.bryant) wrote :

I'm tracking the Ubuntu SRU for the work-around in https://bugs.launchpad.net/bugs/1782786.

Reviewed: https://review.opendev.org/664622
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=18ba6e6e6a7d1294bd1f1ece454f84eb65b219bb
Submitter: Zuul
Branch: master

commit 18ba6e6e6a7d1294bd1f1ece454f84eb65b219bb
Author: Eric Fried <email address hidden>
Date: Tue Jun 11 10:24:11 2019 -0500

    Skip test_check_doubled_words hacking check UT

    The referenced bug has reared its ugly head in python 2.7.15 in (at
    least) releaselevel='final', which is being used in the gate, as well as
    3.6.7+. Disable the test entirely until we can fix or work around the
    bug in the tokenizer.

    The hacking check itself is still in place. This is just disabling the
    unit test that tries to test the hacking check.

    NOTE: Noticed that the hacking check is operating on physical_line,
    which means it'll catch things like:

     "This is the the error"

    but not:

     ("This is the "
      "the error")

    ...which is probably way more likely.

    Change-Id: I8826c3fb89690805baae6b9b7b48985abb8d62d3
    Related-Bug: #1804062

Fix proposed to branch: master
Review: https://review.opendev.org/664940

Changed in nova:
assignee: nobody → Stephen Finucane (stephenfinucane)
status: Confirmed → In Progress
Download full text (3.7 KiB)

Reviewed: https://review.opendev.org/664940
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=f545a25cc443c41dcd9bdd028064c28b53f56037
Submitter: Zuul
Branch: master

commit f545a25cc443c41dcd9bdd028064c28b53f56037
Author: Stephen Finucane <email address hidden>
Date: Wed Jun 12 15:10:59 2019 +0100

    Fix double word hacking test

    At present, 'pycodestyle' feeds the following string into the 'tokenizer'
    library:

        ["'This is the the best comment'"]

    (note the added quotes because this isn't valid Python otherwise)

    On previous versions of Python, this tokenizer would parse the string like so:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'")
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 0 = 'ENDMARKER')

    However, with the fix [1] backported to recent versions of Python, this now
    resolves to:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'")
      (4, '', (1, 30), (1, 31), '')
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 4 = 'NEWLINE', 0 = 'ENDMARKER')

    Typically, 'pycodestyle' will run physical line checks on each line as it
    parses the token:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2036

    For the former case above, the line doesn't include a newline which
    means we never parse a 'NEWLINE' token with a logical line (the fifth
    element of the token tuple) corresponding to our full line. This means
    we don't here but that wasn't an issue previously since there's a
    fallthrough case that handled tokens remaining at the end of the parse:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2114-L2116

    Unfortunately, because we now have an additional newline character to
    parse, one that's on a separate line to our test string no less, we run
    logical checks on it:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2105-L2107

    This is an issue since the logical check wipes stored tokens meaning
    we've nothing to check when we get to the fallthrough case:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2012

    This fixes changes things so that a newline is included (and also adds
    quotes so it's valid Python, but that's mostly unrelated). This means we
    end up with the following instead:

      ["'This is the the best comment'\n"]

    On both Python without the bugfix and with it, this parses as:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'\n")
      (4, '\n', (1, 30), (1, 31), "'This is the the best comment'\n")
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 4 = 'NEWLINE', 0 = 'ENDMARKER')

    Which triggers things in 'pycodestyle' correctly.

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2044-L2046

    This isn't _really_ a fix since there's clearly still a bug in either
    'pycodestyle' or Python (I think the latter, since it's adding a newline
    ...

Read more...

Changed in nova:
status: In Progress → Fix Released

Change abandoned by Lee Yarwood (<email address hidden>) on branch: stable/stein
Review: https://review.opendev.org/664841
Reason: https://review.opendev.org/665147

Download full text (4.0 KiB)

Reviewed: https://review.opendev.org/665147
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=0cb6106b83c33bded9e6cdec7737964c36be8de5
Submitter: Zuul
Branch: stable/stein

commit 0cb6106b83c33bded9e6cdec7737964c36be8de5
Author: Stephen Finucane <email address hidden>
Date: Wed Jun 12 15:10:59 2019 +0100

    Fix double word hacking test

    At present, 'pycodestyle' feeds the following string into the 'tokenizer'
    library:

        ["'This is the the best comment'"]

    (note the added quotes because this isn't valid Python otherwise)

    On previous versions of Python, this tokenizer would parse the string like so:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'")
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 0 = 'ENDMARKER')

    However, with the fix [1] backported to recent versions of Python, this now
    resolves to:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'")
      (4, '', (1, 30), (1, 31), '')
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 4 = 'NEWLINE', 0 = 'ENDMARKER')

    Typically, 'pycodestyle' will run physical line checks on each line as it
    parses the token:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2036

    For the former case above, the line doesn't include a newline which
    means we never parse a 'NEWLINE' token with a logical line (the fifth
    element of the token tuple) corresponding to our full line. This means
    we don't here but that wasn't an issue previously since there's a
    fallthrough case that handled tokens remaining at the end of the parse:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2114-L2116

    Unfortunately, because we now have an additional newline character to
    parse, one that's on a separate line to our test string no less, we run
    logical checks on it:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2105-L2107

    This is an issue since the logical check wipes stored tokens meaning
    we've nothing to check when we get to the fallthrough case:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2012

    This fixes changes things so that a newline is included (and also adds
    quotes so it's valid Python, but that's mostly unrelated). This means we
    end up with the following instead:

      ["'This is the the best comment'\n"]

    On both Python without the bugfix and with it, this parses as:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'\n")
      (4, '\n', (1, 30), (1, 31), "'This is the the best comment'\n")
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 4 = 'NEWLINE', 0 = 'ENDMARKER')

    Which triggers things in 'pycodestyle' correctly.

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2044-L2046

    This isn't _really_ a fix since there's clearly still a bug in either
    'pycodestyle' or Python (I think the latter, since it's adding a newlin...

Read more...

Download full text (3.8 KiB)

Reviewed: https://review.opendev.org/665788
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=4858074c89838eadeb9eaf9f39917e9fb90acd93
Submitter: Zuul
Branch: stable/rocky

commit 4858074c89838eadeb9eaf9f39917e9fb90acd93
Author: Stephen Finucane <email address hidden>
Date: Wed Jun 12 15:10:59 2019 +0100

    Fix double word hacking test

    At present, 'pycodestyle' feeds the following string into the 'tokenizer'
    library:

        ["'This is the the best comment'"]

    (note the added quotes because this isn't valid Python otherwise)

    On previous versions of Python, this tokenizer would parse the string like so:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'")
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 0 = 'ENDMARKER')

    However, with the fix [1] backported to recent versions of Python, this now
    resolves to:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'")
      (4, '', (1, 30), (1, 31), '')
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 4 = 'NEWLINE', 0 = 'ENDMARKER')

    Typically, 'pycodestyle' will run physical line checks on each line as it
    parses the token:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2036

    For the former case above, the line doesn't include a newline which
    means we never parse a 'NEWLINE' token with a logical line (the fifth
    element of the token tuple) corresponding to our full line. This means
    we don't here but that wasn't an issue previously since there's a
    fallthrough case that handled tokens remaining at the end of the parse:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2114-L2116

    Unfortunately, because we now have an additional newline character to
    parse, one that's on a separate line to our test string no less, we run
    logical checks on it:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2105-L2107

    This is an issue since the logical check wipes stored tokens meaning
    we've nothing to check when we get to the fallthrough case:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2012

    This fixes changes things so that a newline is included (and also adds
    quotes so it's valid Python, but that's mostly unrelated). This means we
    end up with the following instead:

      ["'This is the the best comment'\n"]

    On both Python without the bugfix and with it, this parses as:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'\n")
      (4, '\n', (1, 30), (1, 31), "'This is the the best comment'\n")
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 4 = 'NEWLINE', 0 = 'ENDMARKER')

    Which triggers things in 'pycodestyle' correctly.

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2044-L2046

    This isn't _really_ a fix since there's clearly still a bug in either
    'pycodestyle' or Python (I think the latter, since it's adding a newlin...

Read more...

Download full text (3.9 KiB)

Reviewed: https://review.opendev.org/665790
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=dc2963d2a0002d4ddacf50b1f80c470d5de7ec61
Submitter: Zuul
Branch: stable/queens

commit dc2963d2a0002d4ddacf50b1f80c470d5de7ec61
Author: Stephen Finucane <email address hidden>
Date: Wed Jun 12 15:10:59 2019 +0100

    Fix double word hacking test

    At present, 'pycodestyle' feeds the following string into the 'tokenizer'
    library:

        ["'This is the the best comment'"]

    (note the added quotes because this isn't valid Python otherwise)

    On previous versions of Python, this tokenizer would parse the string like so:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'")
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 0 = 'ENDMARKER')

    However, with the fix [1] backported to recent versions of Python, this now
    resolves to:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'")
      (4, '', (1, 30), (1, 31), '')
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 4 = 'NEWLINE', 0 = 'ENDMARKER')

    Typically, 'pycodestyle' will run physical line checks on each line as it
    parses the token:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2036

    For the former case above, the line doesn't include a newline which
    means we never parse a 'NEWLINE' token with a logical line (the fifth
    element of the token tuple) corresponding to our full line. This means
    we don't here but that wasn't an issue previously since there's a
    fallthrough case that handled tokens remaining at the end of the parse:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2114-L2116

    Unfortunately, because we now have an additional newline character to
    parse, one that's on a separate line to our test string no less, we run
    logical checks on it:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2105-L2107

    This is an issue since the logical check wipes stored tokens meaning
    we've nothing to check when we get to the fallthrough case:

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2012

    This fixes changes things so that a newline is included (and also adds
    quotes so it's valid Python, but that's mostly unrelated). This means we
    end up with the following instead:

      ["'This is the the best comment'\n"]

    On both Python without the bugfix and with it, this parses as:

      (3, "'This is the the best comment'", (1, 0), (1, 30), "'This is the the best comment'\n")
      (4, '\n', (1, 30), (1, 31), "'This is the the best comment'\n")
      (0, '', (2, 0), (2, 0), '')

    where (3 = 'STRING', 4 = 'NEWLINE', 0 = 'ENDMARKER')

    Which triggers things in 'pycodestyle' correctly.

      https://github.com/PyCQA/pycodestyle/blob/2.5.0/pycodestyle.py#L2044-L2046

    This isn't _really_ a fix since there's clearly still a bug in either
    'pycodestyle' or Python (I think the latter, since it's adding a newli...

Read more...

This issue was fixed in the openstack/nova 19.0.2 release.

This issue was fixed in the openstack/nova 18.2.2 release.

This issue was fixed in the openstack/nova 17.0.12 release.

This issue was fixed in the openstack/nova 20.0.0.0rc1 release candidate.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.