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

[doc test helper] Better representation of failure for multiple good files #8991

Conversation

Pierre-Sassoulas
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas commented Sep 1, 2023

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Seen in #8960, previously the error message was:

             msg = f"""There should be no warning raised for 'good.py' but these messages were raised:
    - {messages}
    
    See:
    
    """
>           with open(self._test_file[1]) as f:
E           IsADirectoryError: [Errno 21] Is a directory: '/home/runner/work/pylint/pylint/doc/data/messages/c/cell-var-from-loop/good'

As seen in https://github.com/pylint-dev/pylint/actions/runs/6026790597/job/16350544929?pr=8960. Now with this MR it's:

E           AssertionError: There should be no warning raised for 'good.py' but these messages were raised:

E             - cell-var-from-loop (l. 8)

E             

E             See:

E             

E             

E             ________________________________________________________________________________

E             

E             ________________________________________________________________________________

E             /home/pierre/pylint/doc/data/messages/c/cell-var-from-loop/good/new_function.py

E             ________________________________________________________________________________

E             def greet(name):

E                 print(f"Hello, {name}!")

E             

E             

E             def teacher_greeting(names):

E                 for name in names:

E                     greet(name)

E               # <-- /!\ unexpected 'cell-var-from-loop' (maybe in another file ?)/!\

E             

E             teacher_greeting(["Graham", "John", "Terry", "Eric", "Terry", "Michael"])

E             

E             ________________________________________________________________________________

E             /home/pierre/pylint/doc/data/messages/c/cell-var-from-loop/good/functools.partial.py

E             ________________________________________________________________________________

E             import functools

E             

E             

E             def teacher_greeting(names):

E                 for name in names:

E             

E                     def greet():

E                         print(functools.partial("Hello, {name}!".format, name=name)())  # <-- /!\ unexpected 'cell-var-from-loop' (maybe in another file ?)/!\

E             

E                     greet()

E             

E             

E             teacher_greeting(["Graham", "John", "Terry", "Eric", "Terry", "Michael"])

E             

E             ________________________________________________________________________________

E             

E             Only one file is a problem!

E           assert 1 == 0

E            +  where 1 = <bound method Counter.total of Counter({(8, 'cell-var-from-loop'): 1})>()

E            +    where <bound method Counter.total of Counter({(8, 'cell-var-from-loop'): 1})> = Counter({(8, 'cell-var-from-loop'): 1}).total

@Pierre-Sassoulas Pierre-Sassoulas added Maintenance Discussion or action around maintaining pylint or the dev workflow Skip news πŸ”‡ This change does not require a changelog entry labels Sep 1, 2023
@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Merging #8991 (dfe1698) into main (a21f5d3) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8991   +/-   ##
=======================================
  Coverage   95.75%   95.75%           
=======================================
  Files         173      173           
  Lines       18648    18648           
=======================================
  Hits        17856    17856           
  Misses        792      792           

@Pierre-Sassoulas Pierre-Sassoulas changed the title [doc test helper] Better representation of failute for multiple good files [doc test helper] Better representation of failure for multiple good files Sep 1, 2023
doc/test_messages_documentation.py Outdated Show resolved Hide resolved
doc/test_messages_documentation.py Outdated Show resolved Hide resolved
@Pierre-Sassoulas Pierre-Sassoulas force-pushed the better-error-message-for-multiple-good-files-failures branch from a153835 to dfe1698 Compare September 9, 2023 18:20
@Pierre-Sassoulas
Copy link
Member Author

I re-started from scratch because the approach was fubar. New output after refactor (correct this time):

E           AssertionError: There should be no warning raised for '/home/pierre/pylint/doc/data/messages/t/too-many-lines/good' but these messages were raised:

E             

E             

E             

E             In data/messages/t/too-many-lines/good/is_palindrome.py:

E             

E             def is_palindrome(string):  # <-- /!\ unexpected 'too-many-lines' /!\

E                 return string == string[::-1]

E             

E             def is_palindrome(string):  # [too-many-lines]

E                 left_pos = 0

E                 right_pos = len(string) - 1

E                 while right_pos >= left_pos:

E                     if not string[left_pos] == string[right_pos]:

E                         return False

E                     left_pos += 1

E                     right_pos -= 1

E                 return True

E             

E             def is_palindrome(string):  # [too-many-lines]

E                 left_pos = 0

E                 right_pos = len(string) - 1

E                 while right_pos >= left_pos:

E                     if not string[left_pos] == string[right_pos]:

E                         return False

E                     left_pos += 1

E                     right_pos -= 1

E                 return Tru

E             

E             

E             

E             In data/messages/t/too-many-lines/good/is_palindrome_b.py:

E             

E             def is_palindrome(string):  # <-- /!\ unexpected 'too-many-lines' /!\

E                 return string == string[::-1]

E             

E             def is_palindrome(string):  # [too-many-lines]

E                 left_pos = 0

E                 right_pos = len(string) - 1

E                 while right_pos >= left_pos:

E                     if not string[left_pos] == string[right_pos]:

E                         return False

E                     left_pos += 1

E                     right_pos -= 1

E                 return True

E             

E             def is_palindrome(string):  # [too-many-lines]

E                 left_pos = 0

E                 right_pos = len(string) - 1

E                 while right_pos >= left_pos:

E                     if not string[left_pos] == string[right_pos]:

E                         return False

E                     left_pos += 1

E                     right_pos -= 1

E                 return Tru

E             

E           assert not [Message(msg_id='C0302', symbol='too-many-lines', msg='Too many lines in module (22/15)', C='C', category='convention'...nes/good/is_palindrome_b.py', module='good.is_palindrome_b', obj='', line=1, column=0, end_line=None, end_column=None)]

I realized that there's an issue with the "bad" tests, if there's multiple files we need at least one warning for each files, going to open a fix soonish.

@Pierre-Sassoulas
Copy link
Member Author

Check fail probably due to:

/home/pierre/pylint/doc/user_guide/installation/ide_integration/index.rst:23: WARNING: broken link: https://www.vim.org/scripts/script.php?script_id=891 (HTTPSConnectionPool(host='www.vim.org', port=443): Max retries exceeded with url: /scripts/script.php?script_id=891 (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: certificate has expired (_ssl.c:1007)'))))
          pre-commit run --hook-stage push sphinx-generated-doc --all-files || {
            git diff ; \
            echo "Make sure that there are no modifications locally when launching 'make html'" ; \
            exit 1; \
          }

means the message is launched even when git diff return nothing.

@Pierre-Sassoulas
Copy link
Member Author

Merging without approval because it was approved in #9020 (comment)

@Pierre-Sassoulas Pierre-Sassoulas merged commit ee37544 into pylint-dev:main Sep 10, 2023
@Pierre-Sassoulas Pierre-Sassoulas deleted the better-error-message-for-multiple-good-files-failures branch September 10, 2023 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow Skip news πŸ”‡ This change does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants