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

encoding marker may only appear on the first two lines per PEP263 #529

Closed
nedbat opened this issue Nov 2, 2016 · 7 comments
Closed

encoding marker may only appear on the first two lines per PEP263 #529

nedbat opened this issue Nov 2, 2016 · 7 comments
Labels
bug Something isn't working

Comments

@nedbat
Copy link
Owner

nedbat commented Nov 2, 2016

Originally reported by gpshead (Bitbucket: gpshead, GitHub: gpshead)


The file text encoding marker must only be paid attention to if it falls on the first two lines of the file.

(confirmed by testing that a coding marker in a comment on line 3 is ignored in CPython 2.7 and 3.4)

The bug appears to be in https://bitbucket.org/ned/coveragepy/src/eb9f4a94096cca9863352f600994340caf4f6927/coverage/phystokens.py?at=default&fileviewer=file-view-default#phystokens.py-291 phystokens.neuter_encoding_declaration which neuters the first two encoding declarations seen in a file instead of only within the first two lines.


@nedbat
Copy link
Owner Author

nedbat commented Nov 2, 2016

Original comment by gpshead (Bitbucket: gpshead, GitHub: gpshead)


This may be related to issue #443 which was filed showing an invalid coding specifier not within the first two lines in its example. I'm not sure which reason that issue was filed for.

@nedbat
Copy link
Owner Author

nedbat commented Nov 2, 2016

I'm not sure what wrong behavior in coverage.py you are reporting. What happens? I have a test for this case, so I'm not sure what you are seeing, or whether my test is wrong: https://bitbucket.org/ned/coveragepy/src/eb9f4a94096cca9863352f600994340caf4f6927/tests/test_phystokens.py?fileviewer=file-view-default#test_phystokens.py-131

@nedbat
Copy link
Owner Author

nedbat commented Nov 3, 2016

Original comment by Maurice Pud (Bitbucket: moepud, GitHub: moepud)


I think this is easiest to illustrate with some test cases.

#!python

  def testTwoEncodingDeclaration(self):
    """Two encoding declarations in the first two lines.

    This works with the original upstream version of the function.
    """
    input_src = textwrap.dedent(
        u"""\
        # -*- coding: ascii -*-
        # -*- coding: utf-8 -*-
        # -*- coding: utf-16 -*-
        """)
    expected_src = textwrap.dedent(
        u"""\
        # (deleted declaration) -*-
        # (deleted declaration) -*-
        # -*- coding: utf-16 -*-
        """)
    output_src = phystokens.neuter_encoding_declaration(input_src)
    self.assertEqual(expected_src, output_src)

  def testOneEncodingDeclaration(self):
    """One encoding declaration in the first two lines.

    This does NOT work with the original upstream version of the function.
    """
    input_src = textwrap.dedent(
        u"""\
        # -*- coding: utf-16 -*-
        # Just a comment.
        # -*- coding: ascii -*-
        """)
    expected_src = textwrap.dedent(
        u"""\
        # (deleted declaration) -*-
        # Just a comment.
        # -*- coding: ascii -*-
        """)
    output_src = phystokens.neuter_encoding_declaration(input_src)
    self.assertEqual(expected_src, output_src)

@nedbat
Copy link
Owner Author

nedbat commented Nov 4, 2016

I can see that neuter_encoding_declaration is changing more declarations than it needs to. But is there an actual product-level problem? What source files is coverage.py treating incorrectly? I'd like to understand the whole problem.

@nedbat
Copy link
Owner Author

nedbat commented Nov 4, 2016

Original comment by Maurice Pud (Bitbucket: moepud, GitHub: moepud)


Here's a simple test file that passes when run normally but fails when running under coverage. The reason it fails is that the # -*- coding: utf-8 -*- line in src1 gets rewritten to be # (deleted declaration) -*-.

#!python

# -*- coding: utf-8 -*-

import unittest


class FailsUnderCoverageTest(unittest.TestCase):

  def test_fails_under_coverage(self):
    src1 = u"""\
        # -*- coding: utf-8 -*-
        # Just a comment.
        """
    src2 = u"""\
        # -*- coding: utf-8 -*-
        # Just a comment.
        """
    self.assertEqual(src1, src2)


if __name__ == "__main__":
  unittest.main()

@nedbat
Copy link
Owner Author

nedbat commented Nov 5, 2016

Perfect, thanks for the detail. Fixed in 54e15c3a7e85 (bb).

@nedbat
Copy link
Owner Author

nedbat commented Dec 27, 2016

This fix was released as part of Coverage.py 4.3.

@nedbat nedbat closed this as completed Dec 27, 2016
@nedbat nedbat added major bug Something isn't working labels Jun 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant